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

address np printoptions pre/post v1.22 #4486

Merged
merged 3 commits into from
Jan 11, 2022

Conversation

bjlittle
Copy link
Member

@bjlittle bjlittle commented Jan 10, 2022

🚀 Pull Request

Description

This PR addresses the test failures within #4480 when updating to the latest numpy v1.22.0.

We've now dropped support within iris for py37 (#4481) but we still require to support numpy >= v1.19 (NEP29 Support Table). However numpy/numpy#19686 introduced a change that impacts iris.util.format_array due to our use of the internal numpy.core.arrayprint._formatArray function.

This PR maintains our continuing numpy v1.13 legacy array printing format for py38 and numpy >= 1.19.

As a separate concern, it would be wise to commit to addressing #3048, by at least bumping to the more recent numpy v1.21 legacy array printing.


Consult Iris pull request check list

@bjlittle
Copy link
Member Author

At the moment this PR is testing with py38 and numpy 1.21.5.

However, this PR change also supports py38 and numpy 1.22.0.

I propose that if we're happy with this PR, bank it, then I'll create a follow-on PR that incorporates the nox.lock/py38-linux-64.lock from #4480 (and close #4480).

@trexfeathers trexfeathers self-assigned this Jan 10, 2022
@pp-mo pp-mo mentioned this pull request Jan 10, 2022
Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

I have confirmed for myself that this will fix the #4480 failures, and I think I understand what's going on! I just had one comment about avoiding a private import.

lib/iris/util.py Outdated Show resolved Hide resolved
@bjlittle bjlittle requested a review from pp-mo January 10, 2022 17:54
This was referenced Jan 11, 2022
Copy link
Member

@pp-mo pp-mo left a comment

Choose a reason for hiding this comment

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

Good enough for now, as noted in comments.

@pp-mo pp-mo merged commit 55e6a21 into SciTools:main Jan 11, 2022
@bjlittle
Copy link
Member Author

bjlittle commented Jan 11, 2022

Thanks @pp-mo and @trexfeathers 🍻 🥳 💯

There was a lot of history and tech debt associated with this one.

Hopefully it's raised the priority on us committing to resolving #3048 and moving away from what we have at the moment 👍

@bjlittle bjlittle deleted the np-legacy-113-printoptions branch January 11, 2022 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants