search

pandas-dev / pandas-stubs

Public type stubs for pandas
BSD 3-Clause "New" or "Revised" License
233 stars 123 forks source link

Improve handling of deprecations in stubs #870

Open Daverball opened 8 months ago

Daverball commented 8 months ago

This is mostly a meta issue, I've noticed that when upgrading the stubs for version 2.1.0 the deprecated methods/overloads have been immediately removed.

We now have typing_extensions.deprecated, which I think should be preferred in such cases, since it will create vastly more readable error messages, compared to "No overload variant matches argument types" in the case of deprecating index=1 for DataFrame.groupby or "Series[Any] not callable" in the case of renaming DataFrame.applymap.

In the former case we could've added a deprecated overload that still accepts ColumnAxis and in the latter made a copy of the method and applied the decorator to the deprecated method name.

Dr-Irv commented 8 months ago

This is mostly a meta issue, I've noticed that when upgrading the stubs for version 2.1.0 the deprecated methods/overloads have been immediately removed.

We now have typing_extensions.deprecated, which I think should be preferred in such cases, since it will create vastly more readable error messages, compared to "No overload variant matches argument types" in the case of deprecating index=1 for DataFrame.groupby or "Series[Any] not callable" in the case of renaming DataFrame.applymap.

In the former case we could've added a deprecated overload that still accepts ColumnAxis and in the latter made a copy of the method and applied the decorator to the deprecated method name.

I wasn't aware of the new deprecated operator that would work in stub files. Seems that was only accepted a few months ago!

Prior to that, we've had a policy of removing deprecated behavior in the stubs to encourage people to stop using deprecated behavior. Going forward, we can see if we can make @deprecated work within our testing scheme.

@twoertwein Your thoughts?

twoertwein commented 8 months ago

I'm fine accepting PRs that use it! I'm not sure whether that will create more maintenance burden on our side:

xref https://github.com/pandas-dev/pandas-stubs/pull/848#discussion_r1443864683

I think we should definitely use this decorator for breaking changes in major releases (that currently do not warn; pandas 3.0). Currently we silence these cases in the CI but users do not get a heads up.

Daverball commented 8 months ago

If you're worried about maintenance burden, a reasonable compromise could be to only use deprecated in simple cases, such as renaming/removing a method/function/class. In these cases you would have to add stubtest ignore entry anyways, that you later have to remove again, so in terms of effort they seem about the same to me. At least this way you don't have to edit two separate files and stubtest will tell you when it is time to remove the deprecated entity.

I.e. in the case of renaming applymap to map you would use it, but for the other case, where you need to add an additional overload you wouldn't, unless someone already went through the trouble of writing tests for it.