-
Notifications
You must be signed in to change notification settings - Fork 615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make nesting conditionals supported only for Python 3.7+ #4888
Conversation
Signed-off-by: Albert Wolant <awolant@nvidia.com>
CI MESSAGE: [8488829]: BUILD STARTED |
CI MESSAGE: [8488829]: BUILD FAILED |
@@ -533,7 +533,7 @@ def if_stmt(self, cond, body, orelse, get_state, set_state, symbol_names, nouts) | |||
" same set of keys, the values may be different.\n") | |||
|
|||
try: | |||
tree.assert_same_structure(body_outputs, orelse_outputs, check_types=True) | |||
nest.assert_same_structure(body_outputs, orelse_outputs, check_types=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nest.assert_same_structure(body_outputs, orelse_outputs, check_types=True) | |
nest.assert_same_structure(body_outputs, orelse_outputs) |
I think it may not be needed here as nifty_nesting
doesn't support that arg in this call.
CI MESSAGE: [8496068]: BUILD STARTED |
CI MESSAGE: [8496068]: BUILD FAILED |
Signed-off-by: Albert Wolant <awolant@nvidia.com>
Signed-off-by: Albert Wolant <awolant@nvidia.com>
Signed-off-by: Albert Wolant <awolant@nvidia.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this file I used check_nesting_support
to disable the tests if the feature is not supported.
return False | ||
|
||
|
||
if check_nesting_support(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand this turns off the conditional execution for python3.6 now, right?
I'm afraid we cannot do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not. As I stated in the comments for Python 3.6 I pasted old implementation of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. There is else branch. Hard to spot it. Maybe there should be an another module, called legacy and we should import either one or the second to avoid messing up with code?
_conditionals.py:
if check_nesting_support():
from _conditionals_new import *
else:
from _conditionals_legacy import *
# print waring about lack of support for nested data strucutres
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Signed-off-by: Albert Wolant <awolant@nvidia.com>
# if a_val: | ||
# and_output = b() | ||
# else: | ||
# and_output = a_val |
Check notice
Code scanning / CodeQL
Commented-out code
# if a_val: | ||
# or_output = a_val | ||
# else: | ||
# or_output = b() |
Check notice
Code scanning / CodeQL
Commented-out code
# if a_val: | ||
# and_output = b() | ||
# else: | ||
# and_output = a_val |
Check notice
Code scanning / CodeQL
Commented-out code
# if a_val: | ||
# or_output = a_val | ||
# else: | ||
# or_output = b() |
Check notice
Code scanning / CodeQL
Commented-out code
Signed-off-by: Albert Wolant <awolant@nvidia.com>
Signed-off-by: Albert Wolant <awolant@nvidia.com>
Signed-off-by: Albert Wolant <awolant@nvidia.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
CI MESSAGE: [8505031]: BUILD STARTED |
CI MESSAGE: [8505032]: BUILD STARTED |
CI MESSAGE: [8505031]: BUILD FAILED |
CI MESSAGE: [8505032]: BUILD FAILED |
Signed-off-by: Albert Wolant <awolant@nvidia.com>
CI MESSAGE: [8505806]: BUILD STARTED |
CI MESSAGE: [8505807]: BUILD STARTED |
CI MESSAGE: [8505806]: BUILD PASSED |
CI MESSAGE: [8505807]: BUILD PASSED |
…)" This reverts commit fbefa7d.
* dm-tree is not supported in Python 3.6. This PR makes this dependency and the feature it requires for conditional execution available only for Python 3.7+. Signed-off-by: Albert Wolant <awolant@nvidia.com>
Category:
Bug fix
Description:
dm-tree
is not supported in Python 3.6. This PR makes this dependency and the feature it is required for conditional and available only for Python 3.7+.Key points relevant for the review:
Did I run CI for Python 3.6? Did I run the CI for other Python versions?
Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A