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

custom unit formats #1371

Merged
merged 25 commits into from
Oct 9, 2021
Merged

custom unit formats #1371

merged 25 commits into from
Oct 9, 2021

Conversation

keewis
Copy link
Contributor

@keewis keewis commented Aug 14, 2021

This adds a register_unit_format function and implements the currently available formats using that. I'd also like to switch the siunitx code ("Lx") to use the new format, but because that is implemented in a function that expects a Unit object instead of a UnitContainer I still need to figure out a way to make it compatible.

I also renamed the default "" format to "D".

@keewis
Copy link
Contributor Author

keewis commented Aug 15, 2021

this currently fails because Measurements.__format__ assumes dicts in _FORMATS. I guess we should probably move that code to a branch in formatter.

@keewis
Copy link
Contributor Author

keewis commented Aug 17, 2021

actually, maybe this is a case for the Quantity format (which would require register_quantity_format): Measurement is a subclass of Quantity, so the code should not relate to the unit format.

Since it's pretty difficult to do that without breaking anything, I renamed the dict of callables to _FORMATTERS and added back the dict of dict structure for _FORMATS.

@keewis
Copy link
Contributor Author

keewis commented Aug 17, 2021

this still needs proper documentation (e.g. there should be a complete list of formats available, with examples), but I guess that depends on #972

@keewis keewis marked this pull request as ready for review August 17, 2021 14:19
@keewis
Copy link
Contributor Author

keewis commented Aug 17, 2021

I just noticed that _parse_spec really is for quantity format specs, so format_unit doesn't really need the additional complexity.

The tests fail because siunitx_format_unit, which I decided to call from the registered format_latex_siunitx, needs a registry. Not sure how to fix that, maybe by adding a _registry field to UnitsContainer?

@hgrecco
Copy link
Owner

hgrecco commented Aug 17, 2021

Regarding siunitx_format_unit The way we have solve this issues in the past is having a registry parameter. It might make sense here.

pint/formatting.py Show resolved Hide resolved
pint/formatting.py Show resolved Hide resolved
Comment on lines 142 to 145
>>> @pint.register_unit_format("custom")
... def format_custom(unit, registry, **options):
... result = "<formatted unit>" # do the formatting
... return result
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is executed twice (once for pint.formatting.register_unit_format and once for pint.register_unit_format), and because there's no isolation in between the runs, the second run will try to register a pre-existing name. Not sure how to avoid that... does sphinx have a test isolation feature, or should we switch to python -m pytest --doctest-modules and the ipython / jupyter sphinx directives?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as far as I can tell, there's not much we can do here, except switching the doctest runner to pytest (and the doctest blocks in the rst files to ipython / jupyter-execute), to replace autodoc with autosummary, or to make sure autodoc does not generate pages for both pint.register_unit_format and pint.formatting.register_unit_format.

Since all of these are out-of-scope for this PR, I'll convert the example to a code block and we can discuss further changes in a dedicated issue / PR.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, there is no way out.

@jules-ch
Copy link
Collaborator

Good work @keewis looks promising. 👍

@keewis
Copy link
Contributor Author

keewis commented Oct 10, 2021

I didn't bring this up before, but I basically disallowed passing quantity modifiers (just # for now) to Unit.__format__. So

f"{u:#C}"

is a error, but

ureg.default_format = "#C"
f"{u}"

is fine.

This might be a breaking change (it caused a few tests to fail in pint-xarray, but I think I'll just remove those) so it would be good to discuss this. For reference, ignoring those is as simple as calling extract_custom_formats on spec in Unit.__format__, just as we do with default_format.

Edit: if we choose to keep the breaking change I think we should improve the error message: #C is not a format specification could be more informative

khaeru added a commit to IAMconsortium/units that referenced this pull request Nov 11, 2021
pint.formatting.format_unit() changed slightly between pint 0.17 and
0.18; see hgrecco/pint@8c719483, part of hgrecco/pint#1371.

- Discard the "~" format modifier once it has been checked.
- Discard any leading ":".
- Adjust docstring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Format without space between magnitude and unit formats, documentation and custom formats
3 participants