Skip to content

ENH: Add strip/lstrip/rstrip ufuncs for unicode and bytes - #25152

Merged
ngoldbaum merged 5 commits into
numpy:mainfrom
lysnikolaou:string-ufuncs-strip-v2
Dec 5, 2023
Merged

ngoldbaum merged 5 commits into
numpy:mainfrom
lysnikolaou:string-ufuncs-strip-v2

Conversation

@lysnikolaou

Copy link
Copy Markdown
Member

A note about this: I added a legacy type resolver, in order to be able to handle None as an input argument. Is there a way to do the equivalent of this with a promoter?

@seberg

seberg commented Nov 15, 2023

Copy link
Copy Markdown
Member

Is there a way to do the equivalent of this with a promoter?

We would need to either dig deeper into the resolve_dtypes_raw (it's plausible) and/or figure out a way to make arguments truly optional.
Unfortunately, I am not sure I can condone adding such a hacky old-style resolver. The only reason I didn't start deleting them, is that I am not sure if we have consensus on going ahead with gh-25009 before releasing 2.0 (it might become necessary, although not the part with actually removing things).

@mattip

mattip commented Nov 15, 2023

Copy link
Copy Markdown
Member

I am not sure if we have consensus on going ahead

Can we discuss this at a community meeting?

@lysnikolaou

Copy link
Copy Markdown
Member Author

Do you have any updates on this from the community meeting? Are there any action points for me related to this?

@seberg

seberg commented Nov 17, 2023

Copy link
Copy Markdown
Member

No, I brought it up briefly, but didn't have a clear plan of action. I would even prefer two ufuncs for now (as is done for np.clip, which is different but in the same boat to some degree) over adding this hack, TBH. (I guess strip_whitespace.)

Of course it would be cooler to implement a generic solution, but I can't help with a clear plan of action for that atm, unfortunately.

Maybe we should make a, hopefully brief, meeting with @ngoldbaum to see if we can think of a better approach.

@ngoldbaum

Copy link
Copy Markdown
Member

Sorry I didn't respond to the ping last week. Happy to have a chat, although I'm off Thursday and Friday for thanksgiving.

@lysnikolaou

Copy link
Copy Markdown
Member Author

Let's schedule something for early next week?

@lysnikolaou

Copy link
Copy Markdown
Member Author

@seberg @ngoldbaum This is ready to go on my side.

Comment thread numpy/_core/umath.py Resolved
Comment thread numpy/_core/umath.py Resolved
Comment thread numpy/_core/umath.py Resolved
Comment thread numpy/_core/umath.py Resolved
Comment thread numpy/_core/umath.py Resolved
Comment thread numpy/_core/umath.py Resolved
Comment thread numpy/_core/src/umath/string_buffer.h Resolved
Comment thread numpy/_core/src/umath/string_ufuncs.cpp Resolved
@ngoldbaum

Copy link
Copy Markdown
Member

To follow up on the earlier discussion: Sebastian, Lysandros, and I had a video call where we agreed this was the best approach. I'm planning to merge this once the tests finish. Thanks @lysnikolaou!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants