Skip to content

Make get_sample_data autoload npy/npz files. #15641

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 22, 2020
Merged

Make get_sample_data autoload npy/npz files. #15641

merged 1 commit into from
May 22, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Nov 9, 2019 •
edited
Loading

This makes examples shorter and more to the point by simplifying the
data-load part. A small (standard) backcompat-dance is needed.

Edit: will also depend on #15645.
Edit: now good to go.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer added this to the v3.3.0 milestone Nov 9, 2019
timhoffm
timhoffm reviewed Nov 9, 2019
View reviewed changes
@anntzer anntzer force-pushed the get_sample_data-npy branch from edfcd85 to 19b3427 Compare November 9, 2019 17:32
@anntzer anntzer mentioned this pull request Nov 9, 2019
Merged
6 tasks
@anntzer anntzer force-pushed the get_sample_data-npy branch from 19b3427 to bb0d0e7 Compare November 11, 2019 20:35
@anntzer anntzer force-pushed the get_sample_data-npy branch from bb0d0e7 to 50e95bf Compare December 29, 2019 18:47
@anntzer
Copy link
Contributor Author

anntzer commented Dec 29, 2019

rebased

@anntzer anntzer force-pushed the get_sample_data-npy branch from 50e95bf to 4388154 Compare February 11, 2020 09:57
tacaswell
tacaswell approved these changes Feb 11, 2020
View reviewed changes
Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

I am normally very wary of kwargs that change the return type, but that train has already left the station with this function so I am 👍 on adding the ability to get it as a numpy array. However I am -0 on changing the default to True in the future.

However given that this is primarily a helper function for our documentation I am perfectly happy to have this merged over my concern.

@anntzer
Copy link
Contributor Author

anntzer commented Feb 11, 2020

Well the whole point is to change the default to True in the future; if you don't want to do that I'm not even sure the PR is worth it.
How many people actually want to have the path to our sample data, as opposed to loading our sample data? (Well sure technically we don't know, however I think we can make a reasonable guess...)

@jklymak jklymak requested a review from tacaswell February 12, 2020 05:18
@anntzer
Make get_sample_data autoload npy/npz files.
d7d11a7 
This makes examples shorter and more to the point by simplifying the
data-load part.  A small (standard) backcompat-dance is needed.
@anntzer
Copy link
Contributor Author

anntzer commented May 22, 2020

rebased

@anntzer anntzer force-pushed the get_sample_data-npy branch from 4388154 to d7d11a7 Compare May 22, 2020 09:31
@tacaswell tacaswell merged commit a1c51a0 into matplotlib:master May 22, 2020
@anntzer anntzer deleted the get_sample_data-npy branch May 22, 2020 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timhoffm timhoffm timhoffm approved these changes

@tacaswell tacaswell Awaiting requested review from tacaswell

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.
5 participants
@anntzer @tacaswell @timhoffm @QuLogic @dstansby