Nested values edge case fix #160
Merged
Conversation
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
Contributor
ncilfone
commented
Nov 17, 2021
ncilfone
commented
- Fixes edge case from Nested config values are not updated with provided value in --config file #152 where the default attrs object wasn't getting recursed to set config arg values within nested classes
- Updated docs for cmd line overrides on nested classes
…ssuming to fall back on defaults instead of recursing the config space to set the correct parameters.
…ng recursed to set config arg values within nested classes.
Pull Request Test Coverage Report for Build 1473206918
💛 - Coveralls |
gbmarc1
approved these changes
Nov 17, 2021
ncilfone
added a commit
that referenced
this pull request
Dec 13, 2021
* Nested values edge case fix (#160) * fixes issues wrt more than 2 levels of class nesting references. was ssuming to fall back on defaults instead of recursing the config space to set the correct parameters. * linted * updated docs for cmd line overrides on nested classes * fixed edge case from #152 where the default attrs object wasn't getting recursed to set config arg values within nested classes. * adding test conf file * fixing some tests * some interesting work but breaking the tests * test_command_line pass * some cleaning and refactoring * fix when optional nested value is None * clearner refactor to handle type optionals * "config" cannot be a general argument * fix some more tests and windows path compatible * s3 tests pass on windows * all tests now are passing * dont need the graph in builde space * renamed everything to builder_space and doc string * some cleaning * removing networkx dep * linted * removed dataclasses dep by swapping to a namedtuple * fix-up of some tests. some were missing correct spock classes as *args. Some now raise a different exception with the new refactor * moved help functionality to separate file for readability of the builder class * documentation. almost finished * finished doc strings. linted * files for extra tests * fixing issues with python3.6 which doesn't have the typing alias 'list' yet only 'typing.List' * linted Co-authored-by: mbelanger_explorance <mbelanger@explorance.com>
ncilfone
added a commit
that referenced
this pull request
Dec 13, 2021
* fixes issues wrt more than 2 levels of class nesting references. was ssuming to fall back on defaults instead of recursing the config space to set the correct parameters. * linted * updated docs for cmd line overrides on nested classes * fixed edge case from #152 where the default attrs object wasn't getting recursed to set config arg values within nested classes. * adding test conf file * wonky patch to deal with the monster under the bed that is #152. refactor should be thought about to handle all the parent/child relations in a cleaner manner * WIP: Changing all the nesting deps to be handle via a DAG. Breaks most tests. * fixes optional class refs. * fix signature to deal with tuner -- should resolve all tests in /tune. Probably a bunch of vestigial code still to remove -- base tests still only 43/48 * fixing some tests (#163) * fixing some tests * some interesting work but breaking the tests * test_command_line pass * some cleaning and refactoring * fix when optional nested value is None * clearner refactor to handle type optionals * "config" cannot be a general argument * fix some more tests and windows path compatible * s3 tests pass on windows * all tests now are passing * dont need the graph in builde space * renamed everything to builder_space and doc string * some cleaning Co-authored-by: mbelanger_explorance <mbelanger@explorance.com> * Re-Improve handling of nested dependencies (#180) * Nested values edge case fix (#160) * fixes issues wrt more than 2 levels of class nesting references. was ssuming to fall back on defaults instead of recursing the config space to set the correct parameters. * linted * updated docs for cmd line overrides on nested classes * fixed edge case from #152 where the default attrs object wasn't getting recursed to set config arg values within nested classes. * adding test conf file * fixing some tests * some interesting work but breaking the tests * test_command_line pass * some cleaning and refactoring * fix when optional nested value is None * clearner refactor to handle type optionals * "config" cannot be a general argument * fix some more tests and windows path compatible * s3 tests pass on windows * all tests now are passing * dont need the graph in builde space * renamed everything to builder_space and doc string * some cleaning * removing networkx dep * linted * removed dataclasses dep by swapping to a namedtuple * fix-up of some tests. some were missing correct spock classes as *args. Some now raise a different exception with the new refactor * moved help functionality to separate file for readability of the builder class * documentation. almost finished * finished doc strings. linted * files for extra tests * fixing issues with python3.6 which doesn't have the typing alias 'list' yet only 'typing.List' * linted Co-authored-by: mbelanger_explorance <mbelanger@explorance.com> * updated readme Co-authored-by: gbmarc1 <marcantoinebelanger.gbmarc@gmail.com> Co-authored-by: mbelanger_explorance <mbelanger@explorance.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.