-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
DOC: Update list of functions that lose units #10848
DOC: Update list of functions that lose units #10848
Conversation
f4e19a9
to
8a686e5
Compare
8a686e5
to
dc84fa5
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.
@rpsfonseca - thanks! But these should not just be added, as the text just above notes the list is for "prior to astropy 4.0 and numpy 1.17". So, there should instead be a new header entry (maybe below this one is best), along the lines of "Numpy array creation functions cannot be used to initialize Quantity". For that, I'd include the explicit example with np.full
that doesn't work (as it is the one most likely to surprise anyone)
@rpsfonseca - Yes to both! |
dc84fa5
to
e039882
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.
Thanks, looks good now!
Since the issue that this PR fixed was labeled as Hacktoberfest, I applied "hacktoberfest-accepted" label here. I am still not fully understanding how the new fest rules work... cc @astrojuanlu Thank you, all! |
thanks @pllim ! I was about to do it myself 😊 |
For future reference, please do not skip CI when there is real code in the docs. This caused real failures, which I am patching it over at #10874 . FYI. |
Thank you for the help! |
@rpsfonseca - no worries, when I reviewed I noticed the few checks but that Question: would you be willing to adjust the |
Perhaps this line in I also added something in https://docs.astropy.org/en/latest/development/docguide.html#adding-a-git-commit . If any of those could be less confusing, please feel free to propose changes as PR. Thanks! |
…-lose-units DOC: Update list of functions that lose units
…nits DOC: Update list of functions that lose units
@mhvk , @eteq and I just found out that the >>> import numpy as np
>>> import astropy
>>> from astropy import units as u
>>> np.__version__
'1.16.5'
>>> astropy.__version__
'4.0.3'
>>> x = 1. * u.m
>>> np.full(10, x)
array([1., 1., 1., 1., 1., 1., 1., 1., 1., 1.]) |
@pllim - I think 1.16 just completely ignored the unit, without even realizing it. |
We dropped numpy 1.16 in #10664 , so it is non-issue for |
…nits DOC: Update list of functions that lose units
Sorry for taking this long, but I opened a PR to address the confusing tips on |
Description
This pull request is to address the missing functions on the list that details which operations lead to Quantities losing units.
Fixes #10845