-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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: Fix reference warning for recarray. #24152
Conversation
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.
Nitpick: if we're adding this to this page, should we organize these submodules alphabetically so that we have
np.char
np.ma
np.rec
?
I put it after 'masked arrays' since they are both array objects and masked arrays are used more. I am not sure they should be alphabetically listed. |
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.
Generally +1 for creating the recarray article for the refguide. I'm hesitant about exposing it in the array
refguide article though, as I don't think recarrays should be used in new code (e.g. pandas is preferrable for tabular data).
doc/source/reference/arrays.rst
Outdated
@@ -45,5 +45,6 @@ of also more complicated arrangements of data. | |||
arrays.nditer | |||
arrays.classes | |||
maskedarray | |||
recordarray |
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.
I'd vote to remove the mention from the top-level array refguide article, just because I don't think recarrays are recommended for new code.
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.
It can be removed. But a "document isn't included in any toctree" warning will be raised. I don't think adding it into routines.rst is a better choice.
Aren't they syntactical sugar over structured arrays to allow access by attribute rather than key? If so, I don't think they are so bad. Maybe the new reference could mention that pandas data frames are much more powerful. |
I added a note to use pandas DataFrame instead. |
Sorry I didn't investigate this more closely before getting involved. What is the exact warning you see? It seems rec array (and masked array) is already rendered under the |
You mean remove numpy.rec module completely? The warning I met is: I think we can improve _arrays.classes.rec part, let routines.array-creation.rec part refer to _arrays.classes.rec part. And remove numpy.rec module reference from these two parts. |
I mean the original warning, before this PR. |
The warning is:
|
Then yes, this
I think would be a better solution. |
Thanks @liang3zy22 |
This is to fix reference warning for module numpy.rec. Put it in draft since I am not sure about current solution.
@rossbar , Please give comment. My current solution is add a new section for "record arrays" just after "masked arrays".
Do you think it is reasonable? If yes, I can add related content in "record arrays" section.