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

Remove unwanted minor tick label for log plots #450

Merged
merged 8 commits into from Apr 21, 2023
Merged

Conversation

pingelit
Copy link
Contributor

Which issue(s) are closed by this pull request?

Closes #71

Changes proposed in this pull request:

  • Disable the minor tick labels for log axis to remove the unwanted tick labels.

@pingelit pingelit added bug For bugs not on the master branch plot labels Mar 16, 2023
@github-actions github-actions bot added this to In progress in Code Backlog Mar 16, 2023
@pingelit
Copy link
Contributor Author

So far I've only added this to the freq plot, due to some open questions:

@f-brinkmann
Copy link
Member

Thanks for checking, I think it resolves the issue. I did not see any effect on linear axes. Applying it only to log. frequency axis might be the cleaner solution as it would maintain as much Matplotlib default behavior as possible.

Copy link
Member

@mberz mberz left a comment

Choose a reason for hiding this comment

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

Just minor things.
I'd also agree to not use this for linearly scaled axes.
We should not close #71 though, or create a new issue, since this only fixes part of the issue. The other problem is that one may end up with a very low number (or on extreme cases none) of major ticks when zooming in on the frequency axis.

Comment on lines 10 to 12
from matplotlib.ticker import (
NullFormatter
)
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
from matplotlib.ticker import (
NullFormatter
)
from matplotlib.ticker import NullFormatter

Comment on lines 11 to 13
from matplotlib.ticker import (
NullFormatter
)
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
from matplotlib.ticker import (
NullFormatter
)
from matplotlib.ticker import NullFormatter

@f-brinkmann
Copy link
Member

Agreed and opened a new issue with a more specific name #451

@pingelit
Copy link
Contributor Author

Just fixed the comments from @mberz.
Before opening the pull request, I want to check if this behavior can be unit tested.

@pingelit pingelit requested a review from mberz March 20, 2023 12:41
f-brinkmann
f-brinkmann previously approved these changes Mar 20, 2023
Code Backlog automation moved this from In progress to Reviewer approved Mar 20, 2023
Code Backlog automation moved this from Reviewer approved to Review in progress Apr 4, 2023
@pingelit pingelit marked this pull request as ready for review April 4, 2023 14:41
@sikersten sikersten self-requested a review April 14, 2023 11:12
Copy link
Member

@f-brinkmann f-brinkmann 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 implementing!

Code Backlog automation moved this from Review in progress to Reviewer approved Apr 18, 2023
@f-brinkmann f-brinkmann merged commit e44da76 into develop Apr 21, 2023
Code Backlog automation moved this from Reviewer approved to Done Apr 21, 2023
@f-brinkmann f-brinkmann deleted the bugfix/log_ticks branch April 21, 2023 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For bugs not on the master branch plot
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants