Issue warning instead of error when parsing Enum while enum support is not enabled #42623
Conversation
facebook-github-bot
added
the
oncall: jit
There was a problem hiding this comment.
I suggested using the warnings module instead because it is used in _recursive.py. I don't know enough about the details of logging libraries but I thought it would be a good idea to be consistent.
Consider also including information in the error message about how to enable experimental support (I remember seeing some env var you can set to do so).
torch/jit/annotations.py
Outdated
There was a problem hiding this comment.
| import logging | |
| import warnings |
There was a problem hiding this comment.
Done.
I think most of the times (by default) logging.warning and warning.warn behave exactly the same, unless configured otherwise:
https://stackoverflow.com/a/37979724
And not prompting user to set the environment variable is on purpose, the feature isn't complete after all.
torch/jit/annotations.py
Outdated
There was a problem hiding this comment.
| logging.warning("Enum support is work in progress, enum class {}" | |
| warnings.warn("Enum support is work in progress, enum class {}" |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@gmagogsfm has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@gmagogsfm merged this pull request in 5d7c3f9. |
💊 CI failures summary and remediationsAs of commit bb41447 (more details on the Dr. CI page):
ci.pytorch.org: 1 failedThis comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 1 time. |
4 participants
Returnning None rather than error matches previous behavior better.
Fixes https://fburl.com/yrrvtes3