Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Support individual styling of major and minor grid through rcParams#29481

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
QuLogic merged 24 commits intomatplotlib:mainfromkonmenel:fix-issue-13919
Jun 24, 2025

Conversation

konmenel
Copy link
Contributor

PR summary

Possiblycloses#13919 issue. Additionally, PR#26121 also tried to solve the same issue but I understand that it is not backwards compatible and there haven't been any updates since 2023. My implementation should be backwards compatible.

I have tested the code with pytest. Some tests failed on my machine, but all were latex tests. The problem is probably my latex installation.

I also tested to code with a small python script that:

  1. Modifies the rcParams at runtime for 3 different scenarios and uses
  2. Uses mstyle files for the same 3 scenario

The results were identical and as expected.

Test

My test script:

# test_grid_rcparams.pyimportmatplotlibasmplimportmatplotlib.pyplotaspltdpi=150#---------------------------------------------# RUNTIME#---------------------------------------------mpl.rcdefaults()mpl.rcParams["axes.grid"]=Truempl.rcParams["ytick.minor.visible"]=Truempl.rcParams["xtick.minor.visible"]=Truempl.rcParams["axes.grid.which"]="both"mpl.rcParams["grid.color"]="red"mpl.rcParams["grid.linewidth"]=1mpl.rcParams["grid.linestyle"]="-"mpl.rcParams["grid.alpha"]=1plt.figure()plt.plot([0,1], [0,1])plt.gcf().savefig("runtime1.png",dpi=dpi)mpl.rcdefaults()mpl.rcParams["axes.grid"]=Truempl.rcParams["ytick.minor.visible"]=Truempl.rcParams["xtick.minor.visible"]=Truempl.rcParams["axes.grid.which"]="both"mpl.rcParams["grid.color"]="red"mpl.rcParams["grid.linewidth"]=1mpl.rcParams["grid.linestyle"]="-"mpl.rcParams["grid.alpha"]=1mpl.rcParams["grid.major.color"]="black"mpl.rcParams["grid.major.linewidth"]=2mpl.rcParams["grid.major.linestyle"]=":"plt.figure()plt.plot([0,1], [0,1])plt.gcf().savefig("runtime2.png",dpi=dpi)mpl.rcdefaults()mpl.rcParams["axes.grid"]=Truempl.rcParams["ytick.minor.visible"]=Truempl.rcParams["xtick.minor.visible"]=Truempl.rcParams["axes.grid.which"]="both"mpl.rcParams["grid.color"]="red"mpl.rcParams["grid.linewidth"]=1mpl.rcParams["grid.linestyle"]="-"mpl.rcParams["grid.alpha"]=1mpl.rcParams["grid.minor.color"]="red"mpl.rcParams["grid.minor.linewidth"]=0.5mpl.rcParams["grid.minor.linestyle"]=":"mpl.rcParams["grid.minor.alpha"]=0.5plt.figure()plt.plot([0,1], [0,1])plt.gcf().savefig("runtime3.png",dpi=dpi)#---------------------------------------------# MSTYLE FILE#---------------------------------------------mpl.rcdefaults()plt.style.use("./style1.mstyle")plt.figure()plt.plot([0,1], [0,1])plt.gcf().savefig("mstyle1.png",dpi=dpi)mpl.rcdefaults()plt.style.use("./style2.mstyle")plt.figure()plt.plot([0,1], [0,1])plt.gcf().savefig("mstyle2.png",dpi=dpi)mpl.rcdefaults()plt.style.use("./style3.mstyle")plt.figure()plt.plot([0,1], [0,1])plt.gcf().savefig("mstyle3.png",dpi=dpi)
# style1.mstyleaxes.grid: Trueytick.minor.visible: Truextick.minor.visible: Trueaxes.grid.which: bothgrid.color: redgrid.linewidth: 1grid.linestyle: -grid.alpha: 1
# style2.mstyleaxes.grid: Trueytick.minor.visible: Truextick.minor.visible: Trueaxes.grid.which: bothgrid.color: redgrid.linewidth: 1grid.linestyle: -grid.alpha: 1grid.major.color: blackgrid.major.linewidth: 2grid.major.linestyle: :
# style3.mstyleaxes.grid: Trueytick.minor.visible: Truextick.minor.visible: Trueaxes.grid.which: bothgrid.color: redgrid.linewidth: 1grid.linestyle: -grid.alpha: 1grid.minor.color: redgrid.minor.linewidth: 0.5grid.minor.linestyle: :grid.minor.alpha: 0.5

Results

mstyle1
mstyle2
mstyle3
runtime1
runtime2
runtime3

PR checklist

ompaiano and MEMAndersen reacted with heart emoji
Copy link

@github-actionsgithub-actionsbot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join uson gitter for real-time discussion.

For details on testing, writing docs, and our review process, please seethe developer guide

We strive to be a welcoming and open project. Please follow ourCode of Conduct.

@timhoffmtimhoffm changed the titleFIX: issue #13919Support individual styling of major and minor grid through rcParamsJan 27, 2025
Copy link
Member

@timhoffmtimhoffm left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The solution is surprisingly simple. I would have thought that there are more complications because of the concurrent settings and the need for backward compatibility.

The only improvement needed is the clearer distinction on the color and linewidth parameter values between "none" andNone.

@timhoffm
Copy link
Member

One related aspect to think about: Do we anticipate that we'll need a distinction between x and y axis as well? Because if so, we should directly introduce these.

timhoffm pushed a commit to timhoffm/matplotlib that referenced this pull requestJan 27, 2025
The `*_or_None` validators have accepted any capitalization of thestring "none" as input. This is overly permissive andwill likely lead to conflicts in the future because we cannotdistinguish between resolving to `None` and `"none"` which may be needfor some parameters in the future.Inspired bymatplotlib#29481.
@konmenel
Copy link
ContributorAuthor

konmenel commentedJan 27, 2025•
edited
Loading

I made the suggested changes.

One related aspect to think about: Do we anticipate that we'll need a distinction between x and y axis as well? Because if so, we should directly introduce these.

Can you clarify what you meant by "a distinction between x and y axis"? Do you mean something like

rcParams["grid.major.xaxis.linestyle"]=":"rcParams["grid.major.yaxis.linestyle"]="-"

timhoffm pushed a commit to timhoffm/matplotlib that referenced this pull requestJan 27, 2025
The `*_or_None` validators have accepted any capitalization of thestring "none" as input. This is overly permissive andwill likely lead to conflicts in the future because we cannotdistinguish between resolving to `None` and `"none"` which may be needfor some parameters in the future.Inspired bymatplotlib#29481.
@konmenel
Copy link
ContributorAuthor

konmenel commentedJan 27, 2025•
edited
Loading

Sorry I forgot to update the ".pyi" file

@timhoffm
Copy link
Member

between x and y axis"? Do you mean something like

rcParams["grid.major.xaxis.linestyle"]=":"rcParams["grid.major.yaxis.linestyle"]="-"

Basically yes. Exact spelling t.b.d.

@konmenel
Copy link
ContributorAuthor

konmenel commentedJan 27, 2025•
edited
Loading

Basically yes. Exact spelling t.b.d.

I think this is possible however it might look "ugly". The change will require theTick class to know which child (XTick,YTick) was instantiated since the lines are created in the parent class.

One possible way is by accessing theself.__name__ attribute which should either be"xtick" or"ytick" since the base class is never instantiated. However, this might cause problems if this attribute ever changes which might be hard to debug. It is also possible to add a new attribute to theTick class which will default toNone and will be set by the child class to be used explicitly for this purpose so that we don't use the__name__ attribute.

Another way is to add an extra required argument to the constructor ofTick, probably an enum or a bool. For instance, an argumentaxis that will be either 1 or 2 ifXTick orYTick has called theTick.__init__ method, respectively.

Any thoughts on those?

@timhoffm
Copy link
Member

We already rely onTick.__name__ see

name=self.__name__
major_minor="major"ifmajorelse"minor"
self._size=mpl._val_or_rc(size,f"{name}.{major_minor}.size")
self._width=mpl._val_or_rc(width,f"{name}.{major_minor}.width")
self._base_pad=mpl._val_or_rc(pad,f"{name}.{major_minor}.pad")
color=mpl._val_or_rc(color,f"{name}.color")
labelcolor=mpl._val_or_rc(labelcolor,f"{name}.labelcolor")
ifcbook._str_equal(labelcolor,'inherit'):
# inherit from tick color
labelcolor=mpl.rcParams[f"{name}.color"]
labelsize=mpl._val_or_rc(labelsize,f"{name}.labelsize")

It's ok to use that.

Spelling: We have "xaxis", "xticks", "xscale" as axis-dependent names (but they are all also used in the python interface). We do not have any xgrid or similar names. Options:

  • "xgrid.major.linestyle" - in analogy to "xticks"
  • "grid.xaxis.major.linestyle" / "grid.x.major.linestyle" - still put it under the "grid" top level group
  • "xtick.major.grid.linestyle" - put everything under the corresponding tick
  • "xaxis.grid.major.linestyle" - put everything under the corresponding axis

None of them look like the obvious right choice to me.

@konmenel
Copy link
ContributorAuthor

Out of the four, I prefer two:

  • "xtick.major.grid.linestyle" - many options already exist under "xtick.major.*" so it is logical.
  • "grid.xaxis.major.linestyle" - configures similar to both axis. Also, it will be in a similar place in the matplotlibrc making easier to find if you are looking for it.

I will try to implement the second one for now. I can change the key later.

timhoffm reacted with thumbs up emoji

timhoffm added a commit to timhoffm/matplotlib that referenced this pull requestJan 27, 2025
The `*_or_None` validators have accepted any capitalization of thestring "none" as input. This is overly permissive andwill likely lead to conflicts in the future because we cannotdistinguish between resolving to `None` and `"none"` which may be needfor some parameters in the future.Inspired bymatplotlib#29481.
@konmenel
Copy link
ContributorAuthor

I implemented the x and y axis distinction however I came across something that needs further clarification. Right now "grid.xaxis.major.linestyle" is a valid key but "grid.xaxis.linestyle" is not. Should it be a valid option as well? If so it is unclear which of "grid.major.linestyle"and "grid.xaxis.linestyle" should take precedence.

@timhoffm
Copy link
Member

No, we don't need "grid.xaxis.linestyle". If starting from scratch, I'd just have the granular settings "grid.xaxis.major.linestyle" etc. After all, these are advanced style configuration options, and if somebody cares to adapt them, it's ok to write out the details. We keep the high-level ones "grid.linestyle" for backward compatibility (and it is a nice convenience) but we don't need the middle-ground.

Copy link
Member

@timhoffmtimhoffm left a comment

Choose a reason for hiding this comment

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

Looks good. Please add a What's New note according tohttps://matplotlib.org/devdocs/devel/api_changes.html#what-s-new-notes

`_val_or_rc` now accept multiple rc names and return val or the first non-None value in rcParams. Returns last rc name if all other are None. Also, simplified code in `Tick` for grid lines creatation
@konmenel
Copy link
ContributorAuthor

I fix the spelling so all tests should pass now. If there is anything else please let me know.

@carlosgmartin
Copy link

@timhoffm Any idea when this might be merged?

@timhoffm
Copy link
Member

This needs a second review by a core developer, and review time is scarce. However, this is flagged for 3.11 and I'm pretty sure it will get the attention in time for the next release.

@konmenel
Copy link
ContributorAuthor

@QuLogic I have made the changes you suggested. If there is anything more I need to do, please let me know.

@QuLogic
Copy link
Member

stubtest is still complaining about_validate_linestyle since you seemed to have changed only one file.

@konmenel
Copy link
ContributorAuthor

Yes, I did it that way because this is how it is in the main branch. I never modified the function_validate_linestyle in the PR, I just removed it fromci/mypy-stubtest-allowlist.txt.

Do you want me to modify the function or revert the change in "ci/mypy-stubtest-allowlist.txt"?

@QuLogicQuLogic merged commit6e51bec intomatplotlib:mainJun 24, 2025
39 of 40 checks passed
@github-project-automationgithub-project-automationbot moved this fromNeeds decision toWaiting for author inFirst Time ContributorsJun 24, 2025
@QuLogic
Copy link
Member

Thanks@konmenel! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.

@QuLogicQuLogic moved this fromWaiting for author toMerged inFirst Time ContributorsJun 24, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@github-actionsgithub-actions[bot]github-actions[bot] left review comments

@QuLogicQuLogicQuLogic approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

Assignees
No one assigned
Projects
Milestone
v3.11.0
Development

Successfully merging this pull request may close these issues.

Impossible to configure minor/major grid line style independently in rcParams
4 participants
@konmenel@timhoffm@carlosgmartin@QuLogic
[8]ページ先頭
©2009-2025 Movatter.jp