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

MAINT: Reflect changes from numpy namespace refactor Part 5 #26664

Closed
wants to merge 1 commit into from

Conversation

mtsokol
Copy link
Contributor

@mtsokol mtsokol commented Aug 31, 2023

Hi!
Here's a PR that reflects changes introduced in numpy/numpy#24587. (The only item that needs to be modified in matplotlib is np.recarray access)

@anntzer
Copy link
Contributor

anntzer commented Aug 31, 2023

While technically correct, I would suggest just dropping the use of recarrays altogether (remove the .view(np.recarray) and replace r.attr by r['attr'] throughout). If we feel like it we could even switch to using the data kwarg (fill_between("date", pricemin, "close", alpha=0.7, data=r) etc.)

@ksunden
Copy link
Member

ksunden commented Aug 31, 2023

As @anntzer points out, we probably don't need recarray at all.

However, if we do keep it, this change is backwards incompatible, and thus would force us to run our doc builds on np 2.0 (and potentially cause confusion for readers of the docs who are using current released numpy)

The direct impact of this change is minimal to us, as all changes are contained within examples, not library code.

There are two problems with this change that make ordering with regards to numpy a) merging and b) releasing 2.0 that may make us want to hold off on merging (or breaking up the change into two phases) possibly until after release:

  • The first is making sure the examples run in our doc build environment

    • This could be worked around with conditional imports, etc
  • The second is the inter-sphinx references to recarray, which are not found currently

    • I'm pretty sure we link against stable numpy docs, so this may hold off until release
    • Merging early will cause our doc builds to constantly fail, even with backwards-compatible example usage.

@jklymak
Copy link
Member

jklymak commented Aug 31, 2023

I would not use recarray, and remove mention of it in our docs.

@mtsokol
Copy link
Contributor Author

mtsokol commented Aug 31, 2023

However, if we do keep it, this change is backwards incompatible, and thus would force us to run our doc builds on np 2.0 (and potentially cause confusion for readers of the docs who are using current released numpy)

@ksunden I think it would be beneficial to remove recarray from the codebase, but in terms of this PR I don't think it's backward incompatible.

recarray is available under np.recarray and np.rec.recarray, so switching to the second option will still work for previous numpy versions. Am I missing anything?

(The purpose of refactoring that piece of NumPy is to define only one place where this class is available)

@ksunden
Copy link
Member

ksunden commented Aug 31, 2023

Okay, I was thinking that rec was a new namespace, but it isn't, so that prong is okay, the intersphinx not seeing it/breaking the doc build is still true though as of right now

timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Sep 4, 2023
Structured numpy arrays are more fundamental than recarrays
and sufficient in all cases.

Superseeds matplotlib#26664.
timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Sep 4, 2023
Structured numpy arrays are more fundamental than recarrays
and sufficient in all cases.

Superseeds matplotlib#26664.
timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Sep 4, 2023
Structured numpy arrays are more fundamental than recarrays
and sufficient in all cases.

Superseeds matplotlib#26664.
timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Sep 4, 2023
Structured numpy arrays are more fundamental than recarrays
and sufficient in all cases.

Superseeds matplotlib#26664.
timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Sep 4, 2023
Structured numpy arrays are more fundamental than recarrays
and sufficient in all cases.

Superseeds matplotlib#26664.
timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Sep 6, 2023
Structured numpy arrays are more fundamental than recarrays
and sufficient in all cases.

Supersedes matplotlib#26664.
@mtsokol mtsokol closed this Sep 12, 2023
timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Sep 14, 2023
Structured numpy arrays are more fundamental than recarrays
and sufficient in all cases.

Supersedes matplotlib#26664.
timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Sep 14, 2023
Structured numpy arrays are more fundamental than recarrays
and sufficient in all cases.

Supersedes matplotlib#26664.
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.

None yet

4 participants