-
Notifications
You must be signed in to change notification settings - Fork 320
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 issues with infered_from and add checks for infered_from and depends_on that are strings #1453
Fix issues with infered_from and add checks for infered_from and depends_on that are strings #1453
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1453 +/- ##
==========================================
+ Coverage 73.79% 73.86% +0.06%
==========================================
Files 92 92
Lines 10374 10410 +36
==========================================
+ Hits 7656 7689 +33
- Misses 2718 2721 +3 |
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.
Great!
|
Mypy: I expected that
But that does not seem to be that simple. |
134de3b
to
4217ba0
Compare
You cannot meaningfully use a List of a Union unfortunately python/mypy#3351 |
Make sure that it fixes the issue observed in v3
This should either be refactored to reduce duplication or simplified to only generate new desc when needed
d1a9e21
to
f68c853
Compare
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.
Great work!
Modulo a few comments above, this looks ready. |
Merge: 3b93f62 778de4d Author: Jens Hedegaard Nielsen <[email protected]> Merge pull request #1453 from jenshnielsen/fix/validate_paramspec_input_infered
Fixes #1448
Perform db upgrade to correct:
In addition:
As noted below I have not found a good way to get mypy to catch this kind of error.
You cannot use
List[Union[ParamSpec, Str]]
as list is an invariant type python/mypy#3351, the current type checking does not catch is since str in it self is a Sequence@WilliamHPNielsen could you do a review