-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
ENH: Add digits parameter to EngFormatter for significant figures #30827
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
base: main
Are you sure you want to change the base?
ENH: Add digits parameter to EngFormatter for significant figures #30827
Conversation
…ed digits parameter for significant figure formatting, trim_zeros parameter to control trailing zeros, ensured mutual exclusion with places parameter, implemented boundary rollover (e.g., 999.95 → 1.000 k), added comprehensive tests for new functionality, and maintained backward compatibility. Should address matplotlib#30727
timhoffm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a What's new entry
| "Cannot specify 'places' and 'digits'" | ||
| ) | ||
|
|
||
| if trim_zeros not in ("keep", "trim"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to not make trim_zeros a bool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially chose string values to make the intent more explicit in code (e.g., trim_zeros="trim" reads clearly), but a bool would be simpler. I'm happy to change it to 'trim_zeros=False' to keep trailing zeros (i.e., default) and 'trim_zeros=True' to trim trailing zeros. Would you prefer this bool approach?
lib/matplotlib/ticker.py
Outdated
| """ | ||
| sign = 1 | ||
| fmt = "g" if self.places is None else f".{self.places:d}f" | ||
| fmt = "g" if self.places is None and self.digits is None else None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to the block "# Determine format string based on digits or places" so that all the logic is in one place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing!
|
Please fix the issues marked by ruff and mypy-stubtest. |
…ed digits parameter for significant figure formatting, trim_zeros parameter to control trailing zeros, ensured mutual exclusion with places parameter, implemented boundary rollover (e.g., 999.95 → 1.000 k), added comprehensive tests for new functionality, and maintained backward compatibility. Should address #30727
PR summary
PR checklist
Closes #30727