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

Deprecate rcParams["datapath"] in favor of mpl.get_data_path(). #16417

Closed
wants to merge 1 commit into from

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Feb 5, 2020

The rcParam cannot be meaningfully set by the end user from their
matplotlibrc or Python code.

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 Feb 5, 2020
@anntzer anntzer force-pushed the deprecate-rcdatapath branch 2 times, most recently from 8059bdf to 7c7e3e7 Compare February 5, 2020 13:39
@anntzer anntzer modified the milestones: v3.3.0, v3.2.1 Mar 5, 2020
@anntzer anntzer added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Mar 5, 2020
@anntzer
Copy link
Contributor Author

anntzer commented Mar 5, 2020

Should probably have made it to 3.2 instead due to #16678 (#14401), so I milestoned this to 3.2.1. If we agree about the remilestone I will update the version numbers in the diffs.

@tacaswell
Copy link
Member

👍 but the deprecation notes should go in the 3.2 file.

I don't think this is going to backport cleanly as the "pull defaults from the sample file" logic is not on the 3.2.x branch. It may have to be re-written on that branch.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 10, 2020

that's fine, I'll open a separate pr for 3.2.1.
edit: done at #16722; also moved the deprecation note.

Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

This needs to be synced with changes in #16722 before merging, or just left for the branch merge up.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 16, 2020

let's wait for that other pr to be merged

@anntzer
Copy link
Contributor Author

anntzer commented Mar 18, 2020

looks like all's left after the rebase is just the changelog note?

@tacaswell
Copy link
Member

We should back out most of the changes from #16722

@anntzer
Copy link
Contributor Author

anntzer commented Mar 18, 2020

Ah, I see, done.

The rcParam cannot be meaningfully set by the end user from their
matplotlibrc or Python code.
@QuLogic
Copy link
Member

QuLogic commented Apr 1, 2020

Commit message needs updating.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 1, 2020

I guess I vaguely lost track of the deprecation status here. The diff says "remove the datapath" kwarg, but it's clearly not actually removed, just deprecated? so this just removes the ability to set it in the matplotlibrc. But perhaps in that case we may as well wait for the entire deprecation period to be elapsed and strip out the rcparam all at once?

@QuLogic
Copy link
Member

QuLogic commented Apr 1, 2020

So just wait for 3.4 then, and remove it all?

@anntzer
Copy link
Contributor Author

anntzer commented Apr 1, 2020

I guess so, @tacaswell?

@tacaswell tacaswell modified the milestones: v3.3.0, v3.4.0 Apr 29, 2020
@tacaswell
Copy link
Member

Re-milestoned to 3.4 as this is going to take some time to sort out and remind our selves what is going on and it is better to leave a deprecating thing sitting around warning for an extra cycle that it is to pull the rug out from under someone accidentally.

@tacaswell tacaswell removed the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Apr 29, 2020
@QuLogic QuLogic added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Jan 21, 2021
@QuLogic
Copy link
Member

QuLogic commented Jan 22, 2021

@tacaswell @anntzer can you figure out what needs to be done here?

@anntzer
Copy link
Contributor Author

anntzer commented Jan 22, 2021

Are we fine with completely stripping out rcParams["datapath"] for 3.4? if so I can post a PR.

@@ -179,6 +179,7 @@ rcParams
- The ``pgf.debug``, ``verbose.fileo`` and ``verbose.verbose.level`` rcParams,
which had no effect, have been removed.
- Support for setting :rc:`mathtext.default` to "circled" has been removed.
- The ``datapath`` rcParam has been removed. Use `.get_data_path` instead.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- The ``datapath`` rcParam has been removed. Use `.get_data_path` instead.
- The ``datapath`` rcParam is ignored. Use `.get_data_path` instead.

@tacaswell
Copy link
Member

Yes to stripping out datapath for 3.4, we have given fair warning on this.

@anntzer
Copy link
Contributor Author

anntzer commented Jan 22, 2021

Superseded by #19340.

@anntzer anntzer closed this Jan 22, 2021
@anntzer anntzer deleted the deprecate-rcdatapath branch January 22, 2021 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants