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

Impossible to use any variant of ".real/.imag" without warning #27610

Closed
h-vetinari opened this issue Jul 26, 2019 · 9 comments · Fixed by #27651

Comments

@h-vetinari
Copy link
Contributor

commented Jul 26, 2019

After #27106, Series.{real,imag} raise a deprecation warning, which I guess is fair enough from the POV of getting rid of things that came originally from subclassing ndarray.

>>> pd.Series([1, 2, 3]).real
__main__:1: FutureWarning: `real` has be deprecated and will be removed in a future verison
array([1, 2, 3], dtype=int64)

However, due to the way numpy and pandas still interact (with a lot of the __array__ stuff behind the scenes), it is now also impossible to use np.{real,imag} without warning:

>>> np.real(pd.Series([1, 2, 3]))
C:\ProgramData\Miniconda3\envs\pandas-dev\lib\site-packages\numpy\lib\type_check.py:161: FutureWarning: `real` has be deprecated and will be removed in a future verison
  return val.real
array([1, 2, 3], dtype=int64)

I'm fine with whichever way is recommended, but there should be at least one way that does not raise warnings...?

@mroeschke @jreback

PS. One might also use this chance to fix a typo: "[...] has been deprecated [...]"

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2019

What can pandas do to avoid the warning from np.real(Series)? I don't see any option.

I'd recommend np.real(Series.to_array())

@h-vetinari

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

What can pandas do to avoid the warning from np.real(Series)? I don't see any option.

The warning comes from pandas, not numpy:

def real(self):

Is there a way to distinguish a call np.real(Series) from Series.real? TBH I don't know the exact path of resolution for that well enough (through all that __array__-stuff). But I'm kind of imagining a hook that is only set through the Series-method, but not the np-array-route. Then it would be easy not to raise in the latter case.

I'd recommend np.real(Series.to_array())

That recommendation could be part of the warning?

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2019

The warning comes from pandas, not numpy:

Right. What I meant was np.real(arraylike) does something like if hasattr(arraylike, 'real'): return array like.real which will necessarily emit a warning, right?

@h-vetinari

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

Thanks for the explanation. In this case, I agree that it's hard to actually do something.

I'm kind of wondering why this deprecation was necessary at all, but I guess amending the warning to inform the user of np.real(Series.to_array()) of Series.to_array().real would be useful?

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2019

@h-vetinari

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

Are you interested in working on it?

That's trivial. Will try to do so later.

@jorisvandenbossche

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

I think the only way to resolve np.real(series) not giving the warning is to use __array_function__ to prevent numpy accessing the attribute (the protocol should have priority, I think). But having a __array_function__ is still somewhat far off I think, and we are going to remove the deprecation in 1.0, an update of the message seems best.

@h-vetinari

This comment has been minimized.

Copy link
Contributor Author

commented Jul 30, 2019

@jorisvandenbossche
Thanks for the info. I think after having bumped the minimum numpy to 1.13.3, we should be getting closer to much of the __array__ stuff. Maybe it just needs to be tried?

we are going to remove the deprecation in 1.0

Once upon a time, the plan was to have no deprecations in 0.25 - I think it might be a bit short to remove the deprecations of 0.25 in 1.0...?

@jorisvandenbossche

This comment has been minimized.

Copy link
Member

commented Jul 30, 2019

I think after having bumped the minimum numpy to 1.13.3, we should be getting closer to much of the array stuff. Maybe it just needs to be tried?

This needs the __array_funcion__ protocol, not __array__ or __array_ufunc__, and that protocol is much newer / experimental / has a large coverage to implement.

Once upon a time, the plan was to have no deprecations in 0.25 - I think it might be a bit short to remove the deprecations of 0.25 in 1.0...?

Ah, I assumed those deprecations were done earlier together with #20419

But yes, that is something we need to discuss. I think currently the implicit idea was to actually remove them, and basically say: "first upgrade to 0.25.0 to see all deprecation warnings and then upgrade to 1.0" (although, who would read such a recommendation ..). But I don't think we have this explicitly discussed somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.