DOC: clarify fromstring requires sep argument - #32798
michaelzhan1 wants to merge 2 commits into
Conversation
tylerjereddy
added
the
sustain-2026
mattip
left a comment
mattip
left a comment
There was a problem hiding this comment.
Thanks for the PR. This doesn't fix the weird np.dtype(np.dtype) call path creating the error when a dtype class is fed to the function. Is that intentional? If so the PR should not close the issue.
| "fromstring() requires a non-empty 'sep' argument. Its binary mode" \ | ||
| " was removed, use frombuffer() instead."); |
There was a problem hiding this comment.
AI tries to preserve historical context when none is needed. Restate this without the "was removed", maybe something like
| "fromstring() requires a non-empty 'sep' argument. Its binary mode" \ | |
| " was removed, use frombuffer() instead."); | |
| "fromstring() requires a non-empty 'sep' argument. See frombuffer()"); |
There was a problem hiding this comment.
If I include a mention to frombuffer at all, then I'd like to keep a reference to a binary input, since otherwise the pointer to frombuffer seems to not have specific motivation. Maybe something like
fromstring() requires a non-empty 'sep' argument. See frombuffer() for binary inputs.
But since the fromstring docs describe the binary mode change and frombuffer recommendation, maybe I can just simplify it all to (and maybe leave a comment in-code explaining why?)
fromstring() requires a non-empty 'sep' argument.
|
No, it doesn't address the To me, it feels like #27038 covers a few topics, one of which this PR attempts to cover. Is the general standard within NumPy to break this issue into multiple smaller issues, or to leave this issue and resolve it across multiple PRs? |
|
The two are indeed very different, so let's leave the issue open and leave this a doc-only PR. |
|
Tagging @InessaPawson so that this PR can be added to the NF sprint tracker |
github-project-automation
Bot
moved this to Open
in NF Sustaining Open Source Series 2026
This branch has not been deployed
4 participants
PR summary
Addresses one issue mentioned in #27038, which describes that
np.fromstring's documentation doesn't match its behavior when omittingsepor passing insep=''(the binary mode behavior was removed in release 2.3.0).I updated the
fromstringdocs by:dtypeargument, since the functionality was removedsepargument, and clarifying that it must not be emptydeprecatedcallout to aversionchangedcalloutValueErrorconditionI also updated the
ValueErrormessage itself to clearly state thatsepis required and must not be empty.First time contributor introduction
Hi! I'm Michael from Bloomberg, participating in the NumFOCUS Open Source program. I use NumPy mostly for personal projects nowadays, but have used it for work and school projects in the past.
AI Disclosure
I used Claude Opus 5.5 to help understand parts of the codebase relevant to the listed issue, particularly how the C functions are exposed and how the documentation is generated. I also used Claude to review the changes locally as a sanity check to increase confidence (for myself) in the completeness of my changes. The code, documentation, and this PR description are all hand-written. I will participate in the PR manually.