Misc fixes/improvements #573
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
So if you used `orient="LR"` it would get lost if you passed in graph_attrs and it didn't have the appropriate value. This fixes that and fixes the unit test and makes sure it's deterministic.
We introspect the dataframe schema a little to capture those aspects. The datatypes are converted to strings to avert any serialization issues.
To have proper project name and version.
Apply Sweep Rules to your PR?
|
So that people can Try Hamilton :)
Makes it explicit that Hamilton is a first party package.
The problem here was that when we pushed materializers we introduced a regression where somehow input node types got treated differently. The bug here was that "input" nodes are created at add dependency time, thus if two functions request the same input, but provide different types things would correctly error, but in this case in #575 the types were compatible. So the design choice here was to: 1. Check that if one is the subset of the other, then we allow it. 2. We then make the subset type the type of the node. 3. We attach originating functions to ensure we can create a good error message. This has the side-effect of propagating all the way through to Variables and such. 4. The mutating node functions are scoped to only work if the Node is deemed External, that way we don't use that code path inadvertently in the future. 5. We fix an assumption in visualization that assumed inputs didn't have functions. Note: the input/external/user-defined node creation should really be pulled out into a separate step, and not created dynamically in the add_dependency part.
So the git commit hook for isort was running with different configuration from the pre-commit CI run. This (a) fixes files to be consistent with (b) the configuration now provided -- e.g. what is first party, and what is local. This should hopefully now prevent any conflicts from reoccurring.
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.
Fixes/improvees a few things:
Changes
How I tested this
Notes
Checklist