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
DM-36116: Add docs for ellipsis workaround. #138
Conversation
Codecov Report
@@ Coverage Diff @@
## main #138 +/- ##
==========================================
+ Coverage 92.12% 92.13% +0.01%
==========================================
Files 44 44
Lines 2729 2733 +4
==========================================
+ Hits 2514 2518 +4
Misses 215 215
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
These changes look good but now I realize that types.EllipsisType
does exist in python 3.10 -- shouldn't we be using it here if it is defined and only falling back to the existing code if it's an older python?
https://docs.python.org/3/library/types.html#types.EllipsisType and https://docs.python.org/3/library/constants.html#Ellipsis
python/lsst/utils/ellipsis.py
Outdated
behave as expected under ``is`` comparisons and type-narrowing expressions, | ||
such as:: | ||
|
||
v: EllipseType | int |
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.
Typo here. EllipseType.
python/lsst/utils/ellipsis.py
Outdated
such as:: | ||
|
||
v: EllipseType | int | ||
if v is not Ellipse: |
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.
Ellipse?
This replaces an outdated code comment with a module docstring, since it doesn't look possible to actually document the symbols exported by this module.
a286a51
to
78aade4
Compare
This replaces an outdated code comment with a module docstring, since it doesn't look possible to actually document the symbols exported by this module.
Checklist
added a release note for user-visible changes to: I don't think it's user-visibly distinct from DM-36108.doc/changes