Skip to content
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

BUG: application of np.sum on a python iterator should raise an error #23107

Open
clime opened this issue Jan 26, 2023 · 4 comments
Open

BUG: application of np.sum on a python iterator should raise an error #23107

clime opened this issue Jan 26, 2023 · 4 comments

Comments

@clime
Copy link

clime commented Jan 26, 2023

Describe the issue:

E.g.:

>>> list(np.sum(map(lambda x: x, [[1, 2], [1,2]]), axis=0))
[[1, 2], [1, 2]]

Incorrect/unexpected result is returned and no error is raised. I would expect np.sum to do a type check and error out in this case.

NOTE: There is something similar for generators already (although it is just a warning):

>>> np.sum(i for i in range(5))
<stdin>:1: DeprecationWarning: Calling np.sum(generator) is deprecated, and in the future will give a different result. Use np.sum(np.fromiter(generator)) or the python sum builtin instead.

Reproduce the code example:

list(np.sum(map(lambda x: x, [[1, 2], [1,2]]), axis=0))

Error message:

No error

Runtime information:

1.23.5
3.9.16 (main, Dec 7 2022, 01:12:08)
[GCC 11.3.0]

Context for the issue:

No response

@clime clime added the 00 - Bug label Jan 26, 2023
@rgommers
Copy link
Member

The result isn't exactly incorrect. Here is what's happening:

>>> import numpy as np
>>> x = np.asarray(map(lambda x: x, [[1, 2], [1,2]]))
>>> x
array(<map object at 0x7f6e7064fe50>, dtype=object)
>>> x.shape
()
>>> x.sum()
<map object at 0x7f6e7064fe50>

We've had the exact opposite request too - namely to make NumPy functions work on iterators. This is all a big design mistake and it would have been much better never to implement this. But it's not a bug.

Now, can we deprecate it? Not sure. Probably though. It could be deprecated in 1.25.0 and then removed in 2.0. If you could do some digging to find the discussion where it was added, that would probably be helpful in figuring out the impact of a deprecation here.

@clime
Copy link
Author

clime commented Jan 26, 2023

The result isn't exactly incorrect. Here is what's happening:

>>> import numpy as np
>>> x = np.asarray(map(lambda x: x, [[1, 2], [1,2]]))
>>> x
array(<map object at 0x7f6e7064fe50>, dtype=object)
>>> x.shape
()
>>> x.sum()
<map object at 0x7f6e7064fe50>

We've had the exact opposite request too - namely to make NumPy functions work on iterators. This is all a big design mistake and it would have been much better never to implement this. But it's not a bug.

Now, can we deprecate it? Not sure. Probably though. It could be deprecated in 1.25.0 and then removed in 2.0. If you could do some digging to find the discussion where it was added, that would probably be helpful in figuring out the impact of a deprecation here.

I have just found this: #15625 but there are links to other related issues too.

Small note: also probably expressions like np.asarray(0) should be considered (for possible deprecation).

Thanks for looking at this! For me personally, the stricter behavior, the better.

@rgommers rgommers added this to the 1.25.0 release milestone Jan 27, 2023
@rgommers
Copy link
Member

Thanks! That deprecation was also discussed in gh-15625 indeed, I commented there. For completion here as well, and I added a 1.25.0 milestone to remember.

Deprecation in 1.25.0 and removal in 2.0 seems good to me. Let's see what others think.

@seberg
Copy link
Member

seberg commented May 8, 2023

Since it was milestoned, I tried having a look at it a bit here: main...seberg:numpy:deprecate-iterables happy if anyone wants to look at it more.

Some annoyances is that:

  • Polynomials run into this, which is probably fine, but they run into it in assert_equal tests. Maybe that is OK and those should just be changed.
    • The main issue here is probably the use of iscomplexobj which doesn't like this (a weird function, but unclear that it should fail on this).
  • apply_along_axis test run into it when the function returns a weird object.

So, maybe nothing major, or maybe annoying small things that need an idea for a work-around like the dtype=AllowObject style API that I think had been discussed before.

@seberg seberg added the triage review Issue/PR to be discussed at the next triage meeting label May 16, 2023
@charris charris modified the milestones: 1.25.0 release, 2.0.0 release May 24, 2023
@seberg seberg removed the triage review Issue/PR to be discussed at the next triage meeting label Jul 26, 2023
@seberg seberg modified the milestones: 2.0.0 release, 2.1.0 release Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants