Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add titlecolor in rcParams #14707

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

Merged
merged 9 commits into from
Jul 19, 2019
Merged

Add titlecolor in rcParams #14707

merged 9 commits into from
Jul 19, 2019

Conversation

OriolAbril
Copy link
Contributor

@OriolAbril OriolAbril commented Jul 6, 2019

PR Summary

Extremely simple solution to #14656. I am not sure if this follows the direction you had in mind, I am open to add an extra parameter to rcparams as discussed in the issue, extend (or not) this behaviour to suptitle.

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

@ImportanceOfBeingErnest
Copy link
Member

As had been commented already by @timhoffm, it might be beneficial to introduce a new axes.titlecolor parameter instead. This would avoid breaking existing user styles and some users would totally love to have this anyways. Of course not to break anything with this new parameter either, it might need to take something like "auto" that falls back to whatever the color was before.

@OriolAbril
Copy link
Contributor Author

Should only axes.titlecolor be added then? Or also figure.titlecolor to follow the paralelism between set_title and suptitle?

@OriolAbril OriolAbril changed the title Make ax.set_title use axes.labelcolor Add titlecolor in rcParams Jul 7, 2019
@ImportanceOfBeingErnest
Copy link
Member

Good question. I do not have an opinion on that. Since we do have figure.titlesize and figure.titleweight, it might be more consistent for the figure title to get its own rcParam for color as well. But I wouldn't actually expect it to be used a lot, if at all...

@OriolAbril
Copy link
Contributor Author

How does this look? I tried to check the behaviour of some other parameters that had an 'auto' option, but I am not sure I actually understood how they handle it.

I was also wondering if it would be a good idea to add a call to set_title to one of the subplots in the style sheets reference.

@OriolAbril OriolAbril force-pushed the titlecolor branch 3 times, most recently from 752cb93 to 587f069 Compare July 8, 2019 22:05
@OriolAbril
Copy link
Contributor Author

I think it is ready for review, and unless there are any more requests, merging too.

@tacaswell tacaswell added this to the v3.2.0 milestone Jul 9, 2019
@tacaswell
Copy link
Member

I am 50/50 on using the string "auto" as a sentinel. On one hand it is slightly ambiguous with the string color names, on the other hand we already have precedent for this and using None would be at least as awkward in the code.

@OriolAbril
Copy link
Contributor Author

Whichever you prefer. It won't be difficult to change. I can leave you some time to discuss it and agree on one alternative for the fallback, just let me know and I will use the chosen one.

@OriolAbril
Copy link
Contributor Author

@tacaswell @ImportanceOfBeingErnest Should I modify the sentinel to None instead of 'auto'?

@jklymak
Copy link
Member

jklymak commented Jul 15, 2019

There are plenty of "None"s in the rc files, so I think its a good convention for "fall back on a default"

@OriolAbril
Copy link
Contributor Author

I was changing the code to use None instead of 'auto' and I realized it would yield incoherent results. 'none' is a valid color, meaning "No color", and even though None is not, 'None' and 'NONE' are, therefore, the value in the rctemplate file will be read as a string, lowercased and mapped to 'none' instead of mapped to None (this is done by validate_color in rcsetup.py). Related to #8479.

It could be something different than 'auto', but then it would diverge from lines.markerfacecolor and lines.markeredgecolor.

matplotlibrc.template Outdated Show resolved Hide resolved
@OriolAbril
Copy link
Contributor Author

Can someone rerun the docs tests? They failed in the installation section:

Package libav-tools is not available, but is referred to by another package.
This may mean that the package is missing, has been obsoleted, or
is only available from another source
However the following packages replace it:
  ffmpeg

Package pgf is not available, but is referred to by another package.
This may mean that the package is missing, has been obsoleted, or
is only available from another source

E: Package 'libav-tools' has no installation candidate
E: Package 'pgf' has no installation candidate
E: Unable to locate package otf-freefont
Exited with code 100

It does not look related to my PR

@jklymak
Copy link
Member

jklymak commented Jul 17, 2019

Can you try a rebase? I think there was a problem with CI that was fixed recently.... I think you just need to do:

git fetch upstream
git rebase upstream/master

assuming you have the upstream branch setup

https://matplotlib.org/3.1.0/devel/gitwash/development_workflow.html

@OriolAbril
Copy link
Contributor Author

Anything else left to do? Maybe add figure.titlecolor? I do not think it would be much used though, and axes already have titlepad and titlelocation which figure does not.

@QuLogic
Copy link
Member

QuLogic commented Jul 19, 2019

PRs need two approvals to be merged.

@timhoffm timhoffm merged commit f529ced into matplotlib:master Jul 19, 2019
@timhoffm
Copy link
Member

Thanks, and congratulations on your first contribution to Matploblib! Hope to see you back some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.