DM-35312 Add support for python typing of Fields #79
Conversation
natelust
force-pushed
the
tickets/DM-35312
branch
2 times, most recently
from
f3c582d
to
cb8a4f6
Compare
Codecov Report
@@ Coverage Diff @@
## main #79 +/- ##
==========================================
+ Coverage 84.07% 84.30% +0.22%
==========================================
Files 40 40
Lines 3008 3172 +164
==========================================
+ Hits 2529 2674 +145
- Misses 479 498 +19
Continue to review full report at Codecov.
|
| @@ -60,6 +67,18 @@ | |||
| YamlLoaders += (CLoader,) | |||
| except ImportError: | |||
| pass | |||
| else: | |||
| YamlLoaders = () | |||
| doImport = None | |||
There was a problem hiding this comment.
Isn't this already set from the try/except earlier? I'm also a bit surprised it's not assigned to something else when yaml is available, but maybe that will be explained by code below.
There was a problem hiding this comment.
I feel that this change should be a separate commit. It's probably better to move the setting down from the import check above but it doesn't seem directly related to the other changes.
There was a problem hiding this comment.
We can move it or drop it. This just fell under "a few other changes to make type checking happy". My checker was unable to tell that YamlLoaders would be bound, and was complaining about a possibly unbound variable, which is understandable as it would need to know about the logic in the exception handler triggering setting variables that will later be used in an if statement check.
There was a problem hiding this comment.
Accidentally put my overview comments on the pipe_tasks PR (lsst/pipe_tasks#692). I blame VSCode's GitHub plugin, which I tried to use to do this review with mixed results.
|
@TallJimbo (1) we do use black in pex_config (2) there is a test matrix and you can see that only 3.10 is passing at the moment. |
|
And I don't think @natelust is running pre-commit because you can see that the formatting check is also failing (hence your comment about indenting above). |
natelust
force-pushed
the
tickets/DM-35312
branch
from
891d339
to
f57788a
Compare
Add support for specifying the type of Fields with python class typing syntax. This will ensure the appropriate type is inferred from a Fields __get__ method, type checking when setting with __set__, as well as setting appropriate dtypes where applicable at runtime.
natelust
force-pushed
the
tickets/DM-35312
branch
from
f57788a
to
d87ebfd
Compare
Add support for specifying the type of Fields with python class
typing syntax. This will ensure the appropriate type is inferred
from a Fields get method, type checking when setting with
set, as well as setting appropriate dtypes where applicable at
runtime.