Skip to content

WIP: Make str → bool conversion consistent - #9877

Closed
eric-wieser wants to merge 2 commits into
numpy:mainfrom
eric-wieser:improve-str-bool
Closed

eric-wieser wants to merge 2 commits into
numpy:mainfrom
eric-wieser:improve-str-bool

Conversation

@eric-wieser

@eric-wieser eric-wieser commented Oct 17, 2017 •

Copy link
Copy Markdown
Member

Just to show what a compatibility-breaking fix for #9875 could look like. Let's leave discussions about whether this is the right thing to do in #9875.

Caused by string values being cast to an int first and then a bool, ie arr.astype(int).astype(bool).
This causes crashes on non-numeric strings, and only accepts strings of 0 as false.

Fixes numpy#9847
@charris

charris commented Dec 18, 2020

Copy link
Copy Markdown
Member

@seberg is this covered by your work?

@seberg

seberg commented Dec 18, 2020

Copy link
Copy Markdown
Member

I have not looked into this, just another beehive to stab. There are corner case changes on 1.20 even, but they are not and improvement with respect to this :(.

This might get seriously in the way at some point, considering how inconsistent it is. If there is any chance to prioritize this and rip off the band-aid, then I would be all for it, but it is a difficult decision to make...

Base automatically changed from master to main March 4, 2021 02:04
@charris charris added the 52 - Inactive Pending author response label Apr 6, 2022
@charris charris closed this Apr 6, 2022
@seberg seberg added the 64 - Good Idea Inactive PR with a good start or idea. Consider studying it if you are working on a related issue. label Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

00 - Bug 25 - WIP 51 - In progress 52 - Inactive Pending author response 64 - Good Idea Inactive PR with a good start or idea. Consider studying it if you are working on a related issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants