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

Convert deprecation warnings to errors #482

Merged
merged 25 commits into from
Jan 5, 2024
Merged

Conversation

JessicaS11
Copy link
Member

icepyx has a series of deprecations that are still warnings. Some are new deprecations during the modifications for cloud reading, and some are stale but were never turned into errors prior to planned removal. This PR aims to turn all current deprecations into errors for the v1 release, with planned removal of the errors in a future minor release.

Copy link

github-actions bot commented Dec 20, 2023

Binder 👈 Launch a binder notebook on this branch for commit d04ea3d

I will automatically update this comment whenever this PR is modified

Binder 👈 Launch a binder notebook on this branch for commit 585a815

Binder 👈 Launch a binder notebook on this branch for commit f450398

Binder 👈 Launch a binder notebook on this branch for commit 5bfd3b9

Binder 👈 Launch a binder notebook on this branch for commit 1d5d465

Binder 👈 Launch a binder notebook on this branch for commit 1fada73

Binder 👈 Launch a binder notebook on this branch for commit 6305372

Binder 👈 Launch a binder notebook on this branch for commit 192f107

Binder 👈 Launch a binder notebook on this branch for commit 06284c0

Binder 👈 Launch a binder notebook on this branch for commit 05d241a

Binder 👈 Launch a binder notebook on this branch for commit ed3abf3

Binder 👈 Launch a binder notebook on this branch for commit 706ffe6

Binder 👈 Launch a binder notebook on this branch for commit 94ab0ce

Binder 👈 Launch a binder notebook on this branch for commit c3eb899

Binder 👈 Launch a binder notebook on this branch for commit 49357be

@JessicaS11
Copy link
Member Author

@rwegener2 This is basically ready for review, but I figured we'd want to merge #468 first since that's got some code changes in the same areas and there could be merge conflicts.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@JessicaS11 JessicaS11 marked this pull request as ready for review January 4, 2024 16:16
@JessicaS11 JessicaS11 requested a review from rwegener2 January 4, 2024 16:16
@JessicaS11
Copy link
Member Author

@rwegener2 This is all set for review!

Copy link
Contributor

@rwegener2 rwegener2 left a comment

Choose a reason for hiding this comment

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

Looks good to me! I'm loving all that code in the Read init that can be eliminated. I had a few suggestions, but I had to commit them directly since Github doesn't let you suggest on un-modified code. I tried to make small descriptive commits so you can see what is changing but overall it was just small updates to deprecated docstrings and condensing the deprecation errors.
Nice PR! 🎉

icepyx/core/read.py Outdated Show resolved Hide resolved
icepyx/core/read.py Outdated Show resolved Hide resolved
@JessicaS11
Copy link
Member Author

I'm loving all that code in the Read init that can be eliminated.

Me too! Thanks for the edits.

@JessicaS11 JessicaS11 merged commit c02b249 into development Jan 5, 2024
2 checks passed
@JessicaS11 JessicaS11 deleted the deprecations branch January 5, 2024 16:42
JessicaS11 added a commit that referenced this pull request Jan 5, 2024
JessicaS11 added a commit that referenced this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants