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

DEP: remove PolyBase from np.polynomial.polyutils #18963

Merged
merged 5 commits into from May 11, 2021

Conversation

hollycorbett
Copy link

DEP: Remove PolyBase from np.polynomial.polyutils

Addresses issue #15658 by removing PolyBase and associated error classes with only usages here. Happy to submit alternative PR using getattr approach instead if desired.

@BvB93
Copy link
Member

BvB93 commented May 9, 2021

Can you also remove the exceptions from the stub file?

class PolyError(Exception): ...
class PolyDomainError(PolyError): ...
# NOTE: Deprecated
# class PolyBase: ...

@hollycorbett hollycorbett force-pushed the polybase_deprecated_removal branch 2 times, most recently from 2b38f7c to 360a959 Compare May 9, 2021 17:58
@hollycorbett
Copy link
Author

All done @BvB93 ... thanks for the heads up. Do let me know if I missed anything.

@charris
Copy link
Member

charris commented May 9, 2021

This could use a release note. See doc/release/upcoming_changes for examples. The name should be 18963.expired.rst.

@BvB93
Copy link
Member

BvB93 commented May 9, 2021

All done @BvB93 ... thanks for the heads up. Do let me know if I missed anything.

Nop, no further annotation-related comments from me.

Holly Corbett added 4 commits May 10, 2021 10:05
Addresses issue numpy#15658 by removing PolyBase class.  Happy to submit alternative PR using getattr approach instead if desired.
Additionally removes associated PolyError classes.  These appear to me to be unused outside this file.
@hollycorbett
Copy link
Author

@charris PR updated. Happy to squash before merge if preferred.

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty of adding a few minor touchups to the release note (basically just formatting + mentioning that the Exceptions are also removed).

LGTM - I'm in favor of removing the objects outright. Thanks @hollycorbett !

@hollycorbett
Copy link
Author

@rossbar feedback much appreciated - agree your edit is much better. Thanks! :)

I took the liberty of adding a few minor touchups to the release note (basically just formatting + mentioning that the Exceptions are also removed).

LGTM - I'm in favor of removing the objects outright. Thanks @hollycorbett !

@charris charris merged commit 74a65e1 into numpy:main May 11, 2021
@charris
Copy link
Member

charris commented May 11, 2021

Thanks @hollycorbett .

@hollycorbett hollycorbett deleted the polybase_deprecated_removal branch May 11, 2021 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants