BUG: fix inconsistency with np.array(['']) being truthy - #5967

Sounds good, but what about " \x00" strings? So I think the fix is right but should be moved further down. I.e. to include all spaces which are followed by only "\x00" and nothing else. Which is our rule for giving back things out of the array.

On the other hand, I find the whitespace rule odd, python seems to behave different?

Of course, in python, empty string falsey and any other string truthy. I don't know why numpy considers whitespace strings falsey aswell, but assumed there must be a good reason since it is explicitly checked? I'm happy to fix that too but it seems like a much bigger logical change.

Yeah, it is. Maybe we could be made a FutureWarning, but should probably write the list before that. Considering the inconsistencies, I think it might be a real option to change, but then I tend to think we should weed out this stuff ;). It is likely difficult to find out why it is as it is, it isn't implausible to me that python itself changed the definition at some point.
We can put this in without all that if you make it trailing "\x00" instead of explicit length 1 check (and add a test for it of course). Maybe it is best to do that anyway and then think about FutureWarnings, etc. in a follow up. (also then we can possibly merge it for 1.10. and put off behaviour change to later)

Though one could argue the fix converts some False's to True's that in the end we might prefer to be False ;), but it is more the other way around probably :).

OK, I will remove the explicit length check and look for all trailing nulls. Can you confirm the tests in the last commit are the desired behaviour for now? (They will be failing currently of course)

+1 to FutureWarning from me, though yeah, should email list
On Jun 15, 2015 6:15 AM, "Pauli Virtanen" [email protected] wrote:

Fortran pads strings with whitespace rather than nulls. The strange
truthiness probably somehow derives from that; even though strings don't
have truth value in F.

—
Reply to this email directly or view it on GitHub
#5967 (comment).

To email list and FutureWarning is for if/when the behaviour for whitespace string is changed to match python's behaviour, not for this change, right? Is anything else needed for this PR?

Code seems good to me the " \0 \0" being False thing seems good to me as well.
Yes, no need to mail the list for the current state probably. I am fine with putting this in as is and then thinking about it in a new PR. But if anyone has compatibility concerns, or you are happy to go for the FutureWarning that is also great (though I admit, it is more likely to be put off after 1.10 maybe then).

@homu r=seberg
.
We definitely shouldn't block the bug fix on the other change, so if you're
happy let's just put this in. Plus, this way we can try the new gizmo :-).
On Jun 16, 2015 12:12 AM, "seberg" [email protected] wrote:

Code seems good to me the " \0 \0" being False thing seems good to me as
well.
Yes, no need to mail the list for the current state probably. I am fine
with putting this in as is and then thinking about it in a new PR. But if
anyone has compatibility concerns, or you are happy to go for the
FutureWarning that is also great (though I admit, it is more likely to be
put off after 1.10 maybe then).

—
Reply to this email directly or view it on GitHub
#5967 (comment).

📌 Commit 8749a9a has been approved by seberg

⌛ Testing commit 8749a9a with merge 8c86a0a...

☀️ Test successful - status