-
Notifications
You must be signed in to change notification settings - Fork 121
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
Fix bug that allowed containers to be set as a value in ValueNodes #335
Merged
+123
−23
Merged
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
5b3b9bc
fix and tests
pereman2 ba34ef0
bugfix
pereman2 229ca6c
bugfix update
pereman2 8998e51
test cases for more types of nodes
pereman2 f880d20
update news, move should_set_value, remove test
pereman2 2ce164d
Update 324.bugfix
omry 92fcd06
refactor _set_value, add testcases
pereman2 117e0fa
omegaconf.is_config and clean
pereman2 e0a186d
set value primitive in anynode
pereman2 ae1c497
test covering _set_item_impl with anynode
pereman2 add7b1f
change explanation
pereman2 01e8487
remove print
pereman2 7596772
added test cases for anynode invalid set
pereman2 12849d5
test anynode test_basic_ops_dict
pereman2 0f5cd66
add testcases test_valid_inputs
pereman2 f9d59ff
change error msg, illegaltype
pereman2 a50b6a7
move tests
pereman2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
test anynode test_basic_ops_dict
commit 12849d5c7f78949fc1800d4f995f7016ae0d7b6b
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
these tests should probably be in test_nodes.py
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.
Well this one tests if the node is not replaced. That cannot be done in test_nodes.
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.
But yeah, I missed adding these testcases in test_valid_inputs in test_nodes
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's okay. Please move the tests to test_nodes.