-
-
Notifications
You must be signed in to change notification settings - Fork 11.7k
Fix structured compare #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
I'm inclined to commit this, but I'll wait until you and Pauli agree. I think it might be worth rewriting the history to just have one commit but maybe that's getting a bit too fancy. As to the backport, that is really up to Ralph at this point. If it was simple Python I'd say yes, but with the c source there is always the possibility of introducing subtle errors. |
|
I rewrote the history to one commit. It should have all the changes for the comments you made, but it looks like rewriting the history threw the comments themselves away. |
|
The first release candidate for 1.5.1 is already out, so I don't think it's a good idea to put it in the 1.5.x branch now. As far as I can tell this is also not an especially urgent thing to fix in 1.5.x (please correct me if I'm wrong). |
|
Yeah, it makes good sense to only fix regressions after the RC. I would argue that the chance for adverse interactions is very minimal, because all the C code is inside an if statement which will only be true when it's on its way to computing the wrong answer or crashing anyway, but it's not so urgent so as to override any precaution. |
|
This still allows broadcasting within arrays: While the expected result is scalar The expected result here is I think the correct fix is to check that |
|
This seems to me like it's a bug in the dtype comparison. Instead of Py_NotImplemented, maybe array_richcompare should raise a PyExc_AssertionError if a and b are different shapes? I think it's an error rather than something to implement in the future. Here's what I get for some dtype comparisons which seems quite wrong: It appears to ignore the shape entirely, and just check the itemsize. |
|
Py_NotImplemented is a message to the interpreter to try the object on the other side of the operator to see if it can handle the request, not to be confused with PyExc_NotImplementedError. I think that comparisons of structured type just does a byte comparison if there are no named fields, or so ISTR from some comment in the code. Ignoring the shape in dtypes would seem consistent with that, although tt doesn't seem quite right. I don't understand how comparisons between structured types is supposed to behave. Is it correct to say that broadcasting is not allowed? |
|
Since there were no more comments on Fortran-style array subarray indexing, I went ahead and implemented removal of the special-casing. Unless this behaviour change is objected to, I think the patch should be good to go. |
|
Cleaned-up patch set here: http://github.com/pv/numpy/compare/master...bug/fix_structured_compare I'll try to see tomorrow to commit it. |
|
Hi Pauli, did you mean to reopen this or was there some sort of github glitch? |
|
Yes, reopened as it's not committed yet -- I was busy last week with other stuff. |
|
Thanks, pushed finally. |
# This is the 1st commit message: Add Windows config to GHA # This is the commit message numpy#2: update script [wheel build] # This is the commit message numpy#3: typo [wheel build] # This is the commit message numpy#4: fix typo? [wheel build] # This is the commit message numpy#5: fix linux builds? [wheel build] # This is the commit message numpy#6: typo [wheel build] # This is the commit message numpy#7: add license and pin to windows 2016 # This is the commit message numpy#8: skip tests [wheel build] # This is the commit message numpy#9: pin to windows 2019 instead [wheel build] # This is the commit message numpy#10: try to find out the error on windows [wheel build] # This is the commit message numpy#11: maybe fix? [wheel build] # This is the commit message numpy#12: maybe fix? [wheel build] # This is the commit message numpy#13: fix? [wheel build] # This is the commit message numpy#14: cleanup [wheel build]
commit 30228578ac65ec6b90961700a5783c23d8e1b879
Author: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Date: Wed Nov 17 16:21:27 2021 -0800
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Update LICENSE_win32.txt
Update LICENSE_win32.txt
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Update LICENSE_win32.txt
Update LICENSE_win32.txt
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Update LICENSE_win32.txt
Update LICENSE_win32.txt
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Update LICENSE_win32.txt
Update LICENSE_win32.txt
commit 4bd12df
Author: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Date: Mon Nov 15 17:28:47 2021 -0800
# This is a combination of 14 commits.
# This is the 1st commit message:
Add Windows config to GHA
# This is the commit message numpy#2:
update script [wheel build]
# This is the commit message numpy#3:
typo [wheel build]
# This is the commit message numpy#4:
fix typo? [wheel build]
# This is the commit message numpy#5:
fix linux builds? [wheel build]
# This is the commit message numpy#6:
typo [wheel build]
# This is the commit message numpy#7:
add license and pin to windows 2016
# This is the commit message numpy#8:
skip tests [wheel build]
# This is the commit message numpy#9:
pin to windows 2019 instead [wheel build]
# This is the commit message numpy#10:
try to find out the error on windows [wheel build]
# This is the commit message numpy#11:
maybe fix? [wheel build]
# This is the commit message numpy#12:
maybe fix? [wheel build]
# This is the commit message numpy#13:
fix? [wheel build]
# This is the commit message numpy#14:
cleanup [wheel build]
commit 444a721
Merge: 376ad69 22448b4
Author: Charles Harris <charlesr.harris@gmail.com>
Date: Mon Nov 15 17:47:23 2021 -0700
Merge pull request numpy#20274 from h-vetinari/fix_15179
TST: Some fixes & refactoring around glibc-dependent skips in test_umath.py
commit 376ad69
Merge: b75fe57 546c47a
Author: Charles Harris <charlesr.harris@gmail.com>
Date: Mon Nov 15 17:31:41 2021 -0700
Merge pull request numpy#20327 from seberg/rename-interpolation
BUG,DEP: Fixup quantile/percentile and rename interpolation->method
commit 546c47a
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Mon Nov 15 16:13:50 2021 -0600
DOC: Fixups for interpolation rename comments from review
Co-authored-by: Charles Harris <charlesr.harris@gmail.com>
commit b75fe57
Merge: 7310c09 cbc25d2
Author: Warren Weckesser <warren.weckesser@gmail.com>
Date: Mon Nov 15 17:27:08 2021 -0500
Merge pull request numpy#20369 from bilderbuchi/missing_diagnostics_newlines
MAINT: Fix newlines in diagnostics output of numpy.f2py.
commit cbc25d2
Author: Christoph Buchner <bilderbuchi@phononoia.at>
Date: Sun Nov 14 08:36:03 2021 +0100
MAINT: Fix newlines in diagnostics output of numpy.f2py.
Linebreaks were not consistently added to errmess/outmess
arguments, which led to very long lines and wrong
concatenation with compiler messages in f2py console output.
commit be15716
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Fri Nov 12 12:10:20 2021 -0600
DOC: Add release not for quantile `interpolation` rename
Also updates the old release note to include the info (and generally
tweaks it a bit)
commit 7d8a8e7
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Fri Nov 12 11:57:22 2021 -0600
DOC: Update percentile/quantile docs
Mainly fixes the method list slightly, tones down the warning a
bit and fixes the link to the paper (I did not realize that the
link failed to work due only because the reference was missing
from nanquantile/nanpercentile).
commit 5bd71fb
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Tue Nov 9 09:48:59 2021 -0600
DOC: Add ticks to quantile interpolation/method error
Co-authored-by: abel <aoun@cerfacs.fr>
commit 0d5fb81
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Mon Nov 8 20:39:50 2021 -0600
DOC: Remove reference to paper from quantile `method` kwarg
Apparently, sphinx does not resolve references to footnotes from
parameter descriptions.
commit 8437663
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Mon Nov 8 18:25:37 2021 -0600
MAINT: Rename interpolation to method in percentile stubs
commit 8cfb6b5
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Mon Nov 8 16:47:27 2021 -0600
TST: Add deprecation testcase for quantile interpolation rename
commit a5ac5a5
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Mon Nov 8 16:41:24 2021 -0600
DOC: Fixup the percentile methods plot
commit 85f3dda
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Mon Nov 8 16:37:41 2021 -0600
BUG: quantile discrete methods ended up using -1 as index sometimes
Also, the closest-observation did not correctly support multiple
quantiles calculated at the same time (broadcasting error).
commit 3993408
Author: Sebastian Berg <sebastian@sipsolutions.net>
Date: Mon Nov 8 15:38:30 2021 -0600
API,DEP: Rename percentile/quantile `interpolation=` to `method=`
commit 22448b4
Author: H. Vetinari <h.vetinari@gmx.com>
Date: Tue Nov 2 16:51:59 2021 +1100
TST: parametrize glibc-version check in test_umath.py
commit 12923c2
Author: H. Vetinari <h.vetinari@gmx.com>
Date: Tue Nov 2 14:21:01 2021 +1100
TST: skip coverage of large elements in sincos_float32 for ancient glibc
Fixes numpy#15179
commit 56268d5
Author: H. Vetinari <h.vetinari@gmx.com>
Date: Tue Nov 2 14:19:24 2021 +1100
TST: use existence of glibc-version to clean up a test
commit 01443e8
Author: H. Vetinari <h.vetinari@gmx.com>
Date: Tue Nov 2 14:18:52 2021 +1100
TST: turn glibc_older_than_2.17 into boolean instead of xfail
this anticipates reuse of this boolean within the test module
commit 9c833bed5879d77e625556260690c349de18b433
Author: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Date: Wed Nov 17 16:21:27 2021 -0800
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Update LICENSE_win32.txt
Update LICENSE_win32.txt
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Update LICENSE_win32.txt
Update LICENSE_win32.txt
Update cibw_test_command.sh
commit 4bd12df
Author: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Date: Mon Nov 15 17:28:47 2021 -0800
# This is a combination of 14 commits.
# This is the 1st commit message:
Add Windows config to GHA
# This is the commit message numpy#2:
update script [wheel build]
# This is the commit message numpy#3:
typo [wheel build]
# This is the commit message numpy#4:
fix typo? [wheel build]
# This is the commit message numpy#5:
fix linux builds? [wheel build]
# This is the commit message numpy#6:
typo [wheel build]
# This is the commit message numpy#7:
add license and pin to windows 2016
# This is the commit message numpy#8:
skip tests [wheel build]
# This is the commit message numpy#9:
pin to windows 2019 instead [wheel build]
# This is the commit message numpy#10:
try to find out the error on windows [wheel build]
# This is the commit message numpy#11:
maybe fix? [wheel build]
# This is the commit message numpy#12:
maybe fix? [wheel build]
# This is the commit message numpy#13:
fix? [wheel build]
# This is the commit message numpy#14:
cleanup [wheel build]
commit 9c833bed5879d77e625556260690c349de18b433
Author: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Date: Wed Nov 17 16:21:27 2021 -0800
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Update LICENSE_win32.txt
Update LICENSE_win32.txt
Add Windows config to GHA
update script [wheel build]
typo [wheel build]
fix typo? [wheel build]
fix linux builds? [wheel build]
typo [wheel build]
add license and pin to windows 2016
skip tests [wheel build]
pin to windows 2019 instead [wheel build]
try to find out the error on windows [wheel build]
maybe fix? [wheel build]
maybe fix? [wheel build]
fix? [wheel build]
cleanup [wheel build]
Update LICENSE_win32.txt
Update LICENSE_win32.txt
Update cibw_test_command.sh
commit 4bd12df
Author: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Date: Mon Nov 15 17:28:47 2021 -0800
# This is a combination of 14 commits.
# This is the 1st commit message:
Add Windows config to GHA
# This is the commit message numpy#2:
update script [wheel build]
# This is the commit message numpy#3:
typo [wheel build]
# This is the commit message numpy#4:
fix typo? [wheel build]
# This is the commit message numpy#5:
fix linux builds? [wheel build]
# This is the commit message numpy#6:
typo [wheel build]
# This is the commit message numpy#7:
add license and pin to windows 2016
# This is the commit message numpy#8:
skip tests [wheel build]
# This is the commit message numpy#9:
pin to windows 2019 instead [wheel build]
# This is the commit message numpy#10:
try to find out the error on windows [wheel build]
# This is the commit message numpy#11:
maybe fix? [wheel build]
# This is the commit message numpy#12:
maybe fix? [wheel build]
# This is the commit message numpy#13:
fix? [wheel build]
# This is the commit message numpy#14:
cleanup [wheel build]
feat: Add vreinterpret intrinsics
Here's the patch for fixing structured array comparisons with multi-dimensional field types.
http://projects.scipy.org/numpy/ticket/1640
Would it make sense to also target it for 1.5.1 or 1.5.2?
Thanks,
Mark