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

Fix specifying number of levels with log contour#27576

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
dstansby merged 8 commits intomatplotlib:mainfromdstansby:log-contour-levels
Apr 30, 2025

Conversation

dstansby
Copy link
Member

@dstansbydstansby commentedDec 28, 2023
edited
Loading

PR summary

When plotting contours with a log norm, passing an integer value to thelevels argument to cap the maximum number of contour levels now works as intended. Partially addresses#19856 and replaces#25149.

I've maually checked that the added test fails before this fix. Testing with the following script:

importmatplotlib.pyplotaspltfrommatplotlib.colorsimportLogNormimportnumpyasnpx,y=np.mgrid[1:10:0.1,1:10:0.1]data=np.abs(np.sin(x)*np.exp(y))forn_levelsinrange(2,20):fig,ax=plt.subplots()im=ax.contourf(x,y,data,norm=LogNorm(),levels=n_levels)cbar=fig.colorbar(im,ax=ax)n_ticks=len(cbar.ax.get_yticklabels())print(f"Requested max{n_levels+1}, got{n_ticks-1}")

Gives on currentmain:

Requested max 3, got 7Requested max 4, got 7Requested max 5, got 7Requested max 6, got 7Requested max 7, got 7Requested max 8, got 7Requested max 9, got 7Requested max 10, got 7Requested max 11, got 7Requested max 12, got 7Requested max 13, got 7Requested max 14, got 7Requested max 15, got 7Requested max 16, got 7Requested max 17, got 7Requested max 18, got 7Requested max 19, got 7Requested max 20, got 7

and with this PR:

Requested max 3, got 3Requested max 4, got 4Requested max 5, got 4Requested max 6, got 4Requested max 7, got 7Requested max 8, got 7Requested max 9, got 7Requested max 10, got 7Requested max 11, got 7Requested max 12, got 7Requested max 13, got 7Requested max 14, got 7Requested max 15, got 7Requested max 16, got 7Requested max 17, got 7Requested max 18, got 7Requested max 19, got 7Requested max 20, got 7

PR checklist

@dstansbydstansby marked this pull request as ready for reviewDecember 28, 2023 21:13
@dstansby
Copy link
MemberAuthor

Thanks - as well as applying your suggestions, I also factored out the logic to set the locator if it's not already set. This allows the long docstrings to be split up, and makes it explicit in_process_contour_level_args where the default locator is being set, instead of hiding it in_autolev.

Comment on lines 1028 to 1029
if args:
# Set if levels manually provided
levels_arg = args[0]
Copy link
Member

@timhoffmtimhoffmApr 9, 2025
edited
Loading

Choose a reason for hiding this comment

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

This could still be made clearer / the current code is still convoluted (but possibly better as a followup PR to not make too many changes and complicate review)

args is not the originalargs but stripped from leading[X, Y], Z. Effectively it can only be empty or contain a single element that containslevels passed positionally (which is supported, but a bit cumbersome to write out as a signature because the optional[X, Y] require to absorb all positional parameters in*args and we therefore have to reunite a possible positional level from args with the possible kwarg level.

The current implementation also has the surprising side effect that you can docontour(Z, levels1, levels=levels2) and the positionallevels1 is sliently ignored.

The only use ofargs here is to pass on a possible positionallevel. We can push the logic out of the function and unite the positional and kwarg paths oflevels in the caller_contour_args - where it belongs logically.

I think eventually we should also makelevels kw-only.contour(Z, 5) orcontour(X, Y, Z, [-1, 0, 1]) are not very readable anyway.

@dstansbydstansby added this to thev3.11.0 milestoneApr 30, 2025
@dstansby
Copy link
MemberAuthor

I've milestoned this as 3.11 because it changes longstanding behaviour (even if it is a bugfix)

@dstansbydstansby merged commit7fd4c67 intomatplotlib:mainApr 30, 2025
40 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@QuLogicQuLogicQuLogic approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

@ksundenksundenAwaiting requested review from ksunden

Assignees
No one assigned
Projects
None yet
Milestone
v3.11.0
Development

Successfully merging this pull request may close these issues.

4 participants
@dstansby@QuLogic@ksunden@timhoffm

[8]ページ先頭

©2009-2025 Movatter.jp