Repository navigation
TYP: add type annotations to a couple utility functions in nddata - #16546
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
eerovaher
left a comment
There was a problem hiding this comment.
Maybe it would be a good idea to introduce a type variable for the datatype of the arrays, like astropy/utils/shapes.py does.
c6f4f80 to
290a719
Compare
|
Thanks for your feedback @eerovaher. I should have addressed all your comments |
|
switching to draft while I figure out how to make sphinx happy |
43de850 to
757646d
Compare
eerovaher
left a comment
There was a problem hiding this comment.
I don't have any typing suggestions left, other than the one remark that requires more familiarity with nddata than I have. Sub-package maintainers should have the final say.
757646d to
f31d2e9
Compare
f31d2e9 to
31886ff
Compare
|
rebased to fix a trivial conflict |
31886ff to
41529ab
Compare
|
rebased again to refresh CI and fix conflicts. This shouldn't block 7.0 (obviously), so I'm going to move it to 7.1 |
|
Hi humans 👋 - this pull request hasn't had any new commits for approximately 4 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then. In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary. If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock. If you believe I commented on this pull request incorrectly, please report this here. |
41529ab to
9fb902a
Compare
|
@mwcraig @bmorris3 @larrybradley can I get eyes on this ? |
|
I'm going to close this pull request as per my previous message. If you think what is being added/fixed here is still important, please remember to open an issue to keep track of it. Thanks! If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here. |
mwcraig
left a comment
There was a problem hiding this comment.
I'm really sorryy I had overlooked this earlier, @neutrinoceros! I think the only change is Complex → Real
|
There are conflicts now. Would need rebase, which will also give you a few more months. |
|
Hi humans 👋 - this pull request hasn't had any new commits for approximately 5 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then. In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary. If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock. If you believe I commented on this pull request incorrectly, please report this here. |
9fb902a to
12fe6a4
Compare
12fe6a4 to
ea62e0e
Compare
7d6e196 to
a39e0f6
Compare
|
sorry about the delay in my reply. I've updated the PR as suggested by @mwcraig but also replaced |
a39e0f6 to
4cd7133
Compare
|
@eerovaher this has approval from the sub-package maintainer. If you rebase, I think we can get this in. |
4cd7133 to
fd0c0e5
Compare
|
I just rebased, fwiw |
Description
A by-product of my reviewing #16544