Final -- Improve handling of nested dependencies #181
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
…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.
…ctor should be thought about to handle all the parent/child relations in a cleaner manner
…. Probably a bunch of vestigial code still to remove -- base tests still only 43/48
* 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 <[email protected]>
* 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 <[email protected]>
|
@gbmarc1 Sorry for all the mentions. This should resolve attribution. Figured it was better to make sure your code contributions were in the blame like it should be. Sorry again for the screw up on my end! |
Pull Request Test Coverage Report for Build 1574822357
💛 - Coveralls |
5 tasks
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.
What does this PR do?
This PR uses simple graph methods to better handle nested dependencies. Also contains a fairly big refactor of some original spaghetti backend code into a much cleaner interface. Mainly contributed by @gbmarc1 via fork. This should close #152.
Gives attribution to @gbmarc1
Checklist