DEP: Use delimiter rather than delimitor as kwarg in mrecords#19921
DEP: Use delimiter rather than delimitor as kwarg in mrecords#19921seberg merged 2 commits intonumpy:mainfrom
delimiter rather than delimitor as kwarg in mrecords#19921Conversation
6d06212 to
5d9352b
Compare
|
Should I add a deprecation warning, in the style of |
|
I suppose considering how exotic this function is, we can likely get away with changing it. But it will have to be a proper deprecation then: give a deprecation warning (with a test), so that we can officially remove it in two versions.
You can probably ignore that... Just give a Deprecation warning, and end on a note about the current version of NumPy (1.22). (You can search the code for |
|
After searching the source code, it appears I have to read NEP 23 — Backwards compatibility and deprecation policy. Thanks for the hints. |
|
Clearly this function is not being used much... |
ae020cf to
71819cb
Compare
bac1c61 to
ac1cc32
Compare
506ce9d to
394d243
Compare
delimiter rather than delimitor as kwarg in mrecords
bashtage
left a comment
There was a problem hiding this comment.
I think the warning could be clarified in case someone uses positional arguments.
80b181e to
e7211af
Compare
seberg
left a comment
There was a problem hiding this comment.
I am happy with this changes right now. Unless anyone has more comments or thinks we just should do this (or feels this is not fringe enough to skip announcing to the mailing list, just comment).
It probably should get a very brief release note for completeness sake though, those are files in doc/release/upcoming_changes with the PR number in the name (there is a README).
One last note: It is possible to pass the argument by position as a work-around for anyone who must support multiple versions.
4f354d5 to
80d3b7b
Compare
|
I have added |
bab8db0 to
a963463
Compare
By keeping both variants of the keyowrd parameter, we achieve backwards compatibility. The old mispelled variant has been removed from the documentation, the new variant does appear in the documentation and using noth variants raises a TypeError. Additionally, the old variant can only be used as a keyword argument, not as a positional argument. Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
a963463 to
cbfa658
Compare
|
I just noticed this corner case, where a deprecation warning is emitted but fromtextfile(fname, delimiter=None, delimitor=";")A bit contrived, but I thought I'd mention it anyway. I can't understand how the CI failure is related to this PR. I have rebased and re-pushed, hopefully the failure will go away by itself. |
|
Yeah, I was aware of that possibility, but passing both is so weird, that I don't think it matters. Lets give this a shot. Thanks @DimitriPapadopoulos. |
First revert the previous commit, too optimistic (#19911).
Then propose a new way to fix the typo, without breaking backwards compatibility.