Skip to content

Enable formatting Quantity units through string format specification - #13050

Merged
mhvk merged 1 commit into
astropy:mainfrom
eerovaher:quantity-unit-fmt
Oct 17, 2022
Merged

Enable formatting Quantity units through string format specification#13050
mhvk merged 1 commit into
astropy:mainfrom
eerovaher:quantity-unit-fmt

Conversation

@eerovaher

@eerovaher eerovaher commented Apr 2, 2022 •
edited by mhvk
Loading

Copy link
Copy Markdown
Member

Description

This pull request enables using unit format names as string format specifiers for Quantity objects, e.g.

>>> from astropy import units as u
>>> f'{1*u.m/u.s:latex_inline}'
'$1 \\; \\mathrm{m\\,s^{-1}}$'

When I was updating the documentation I saw the potential for a few improvements which seemed too small for a separate pull request, so I included them here in a separate commit.
(EDIT: The documentation fixes were moved to a separate pull request)

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the Extra CI label.
  • Is a change log needed? If yes, did the change log check pass? If no, add the no-changelog-entry-needed label. If this is a manual backport, use the skip-changelog-checks label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • Is a milestone set? Milestone must be set but astropy-bot check might be missing; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate backport-X.Y.x label(s) before merge.

@eerovaher eerovaher added this to the v5.1 milestone Apr 2, 2022
@github-actions

github-actions Bot commented Apr 2, 2022

Copy link
Copy Markdown
Contributor

Maybe they couldn't figure out what to make chicken taste like, which is why chicken tastes like everything.

(A special day message.)

@pllim
pllim requested a review from mhvk April 2, 2022 00:23
@nstarman

nstarman commented Apr 2, 2022

Copy link
Copy Markdown
Member

@eerovaher. This is a cool idea.
In #12983 we are refactoring the format specifiers into a mixin class so that cosmology.Parameter can use it as well.
Since cosmology.Parameter has a .unit attribute, it might be worthwhile putting this in the mixin class.

@mhvk

mhvk commented Apr 2, 2022

Copy link
Copy Markdown
Contributor

Hmm, not completely sure: for f'{1*u.m/u.s:latex}', I'd definitely expect the number to be typeset in latex as well, essentially doing q.to_string(format='latex').

A bit more generally, I've long wanted a sensible extension of the mini-language to quantities, angles, times, coordinates, etc. But the difficulty is of course to agree on what is "sensible". I do think we have to try hard to ensure we do not close off avenues -- in this case, linking more explicitly to Quantity.to_string() (and by extension, Angle.to_string(), where we can have real fun with sexagesimal...)

@eerovaher

eerovaher commented Apr 20, 2022

Copy link
Copy Markdown
Member Author

I don't think I'll have time to work on quantity formatting before the next feature freeze, but if merging the documentation updates in d434857 is considered to be a good idea then I can open a separate pull request for that.

@mhvk

mhvk commented Apr 20, 2022

Copy link
Copy Markdown
Contributor

@eerovaher - I'd say go for a small PR with just the doc updates - I just looked at them and they are all good to go in.

@eerovaher eerovaher modified the milestones: v5.1, v5.2 Apr 22, 2022
nstarman
nstarman reviewed May 5, 2022
View reviewed changes
Comment thread astropy/units/quantity.py Resolved
@nstarman nstarman mentioned this pull request May 5, 2022
Closed
5 of 10 tasks
@eerovaher
eerovaher force-pushed the quantity-unit-fmt branch from 37a57ba to 2a4341d Compare July 18, 2022 18:05
@eerovaher

eerovaher commented Jul 18, 2022

Copy link
Copy Markdown
Member Author

Quantity.__format__() now first tries calling Quantity.to_string(), and because the latter recognizes the 'latex' format then the value gets formatted too.

There were 191 failures in the dev version of key dependencies tests, but the same tests have the same number of failures on main as well, so they are not caused by this pull request.

coordinates.Angle is a subclass of Quantity, so it also benefits from this update. Should there be a second change log entry under the coordinates section?

@eerovaher eerovaher removed the Docs label Aug 9, 2022
@nstarman

nstarman commented Sep 8, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

coordinates.Angle is a subclass of Quantity, so it also benefits from this update. Should there be a second change log entry under the coordinates section?

I don't think it's necessary.

@nstarman

nstarman commented Sep 8, 2022

Copy link
Copy Markdown
Member

Re-base for CI bot

@eerovaher
Enable Quantity unit format specifiers
3a35def 
It is now possible to use unit format names as string format specifiers
for a `Quantity`, e.g. `f'{1*u.m/u.s:vounit}'` now produces the string
`'1.0 m.s**-1'`.
mhvk
mhvk approved these changes Oct 16, 2022
View reviewed changes

@mhvk mhvk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking over units PRs for 5.2, I see I missed the rebase here quite a while back. I think here the obvious convenience outweighs my remaining minor worries about exactly how to use the formatting; this seems decently future proof.

@mhvk

mhvk commented Oct 16, 2022

Copy link
Copy Markdown
Contributor

@nstarman, @eerovaher - since this has been quite a while, just a quick check you are both heppy with the PR as is!

@eerovaher

eerovaher commented Oct 17, 2022

Copy link
Copy Markdown
Member Author

No objections to merging from me.

@mhvk

mhvk commented Oct 17, 2022

Copy link
Copy Markdown
Contributor

OK, in it goes! Thanks, @eerovaher!

@mhvk
mhvk merged commit c664582 into astropy:main Oct 17, 2022
@eerovaher
eerovaher deleted the quantity-unit-fmt branch October 17, 2022 21:09
@mhvk mhvk mentioned this pull request Feb 15, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhvk mhvk mhvk approved these changes
@nstarman nstarman nstarman approved these changes

Assignees

No one assigned

Labels

Enhancement units

Projects

None yet

Milestone

v5.2

Development

Successfully merging this pull request may close these issues.

3 participants

@eerovaher @nstarman @mhvk