Skip to content

[EXP] Add UnitSystem class to support custom unit systems - #6065

Closed
adrn wants to merge 4 commits into
astropy:masterfrom
adrn:units/unitsystem
Closed

[EXP] Add UnitSystem class to support custom unit systems#6065
adrn wants to merge 4 commits into
astropy:masterfrom
adrn:units/unitsystem

Conversation

@adrn

@adrn adrn commented May 13, 2017

Copy link
Copy Markdown
Member

This is a port of a UnitSystem object that I use in gala for handling custom systems of units. The basic idea behind a system of units is, at minimum, a set of irreducible bases that all other units can be composed of. However, this class lets you override default composite units for easy conversion. The use case I have is that I often want to transform to a "Galactic" unit system where we represent lengths as kpc, times as Gyr or Myr, but velocities as km/s. The unitsystem can also be accessed like a dictionary, where the keys are the physical types. The following would work with this PR:

>>> usys = UnitSystem(u.kpc, u.Gyr, u.radian, u.Msun, u.km/u.s)
>>> v = 0.2 * u.kpc/u.Gyr
>>> v.to(usys)
<Quantity 0.1955584443346257 km / s>
>>> usys['length'] / usys['time']
Unit("kpc / Gyr")
>>> usys['speed']
Unit("km / s")

There are many details about the implementation that we might want to discuss (do we want singleton classes? do we want to require [length, time, mass, angle] unit bases? etc.).

This would provide a way to fix #703.

At minimum, still needs documentation and a changelog entry...

@adrn adrn added the units label May 13, 2017
@adrn
adrn requested a review from mhvk May 13, 2017 06:18
@pllim pllim changed the title Add UnitSystem class to support custom unit systems [EXP] Add UnitSystem class to support custom unit systems May 13, 2017
@pllim
pllim requested a review from eteq May 13, 2017 13:19
@mhvk

mhvk commented May 14, 2017

Copy link
Copy Markdown
Contributor

@adrn - this looks very nice! Two somewhat related higher-level questions:

  1. Could this be made as an extension of the existing type of "unit systems" (si and cgs);
  2. Would it be better to have a corresponding method to_system instead of repurposing to; it does seem one is asking something different: convert to a new unit (which may or may not be possible), or convert to a preferred unit in a given system (which generally will be possible). Note that Unit already has a to_system method, so it doesn't seem strange to propagate that to Quantity.

@adrn

adrn commented May 15, 2017

Copy link
Copy Markdown
Member Author

@mhvk Thanks for taking a look at all of my PR's that are flying your way :)

  1. I'm not sure. I never really grokked the implementations of si and cgs. To me, they just look like python modules with defined base units. That is, there is no concept of a "preferred representation" of a given physical type that could be different from a composition of bases (as we would want for, at least, the Galactic example above). But if you have an idea for how this PR could be unified with those, I'd love to hear!

  2. Ah, yes, that would be fine by me.

@eteq

eteq commented Jun 16, 2017

Copy link
Copy Markdown
Member

Since we haven't had time to discuss implementation details, I guess this will have to await 3.0 ...

@astropy-bot astropy-bot Bot closed this Nov 21, 2017
@astrofrog astrofrog added the closed-by-bot Closed by stale bot label Nov 21, 2017
@astrofrog

astrofrog commented Nov 21, 2017

Copy link
Copy Markdown
Member

@adrn - this does seem useful, so maybe worth re-opening/rebasing?

@adrn

adrn commented Nov 21, 2017 •
edited
Loading

Copy link
Copy Markdown
Member Author

Yea - @mhvk and I meant to discuss last week but ran out of time. Will try to revisit this once I clear up some stuff in coordinates!

@adrn adrn reopened this Nov 21, 2017
@astrofrog

astrofrog commented Nov 21, 2017

Copy link
Copy Markdown
Member

Ok sounds good - note that this will auto-close again in a week probably (next time the bot runs) unless you rebase or add the 'keep-open' label.

@astropy-bot

astropy-bot Bot commented Nov 21, 2017 •
edited
Loading

Copy link
Copy Markdown

Hi there @adrn 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

I see this is an experimental pull request. I'll report back on the checks once the PR discussion in settled.

If there are any issues with this message, please report them here.

@adrn

adrn commented Nov 21, 2017

Copy link
Copy Markdown
Member Author

Ok - will rebase!

@pllim pllim removed the closed-by-bot Closed by stale bot label Nov 27, 2017
@mwcraig mwcraig mentioned this pull request Jul 26, 2018
Merged
@bsipocz

bsipocz commented Jul 3, 2019

Copy link
Copy Markdown
Member

@adrn - is this still on the table maybe for 4.0?

@astropy astropy deleted a comment from astropy-bot Bot Jul 8, 2019
@astropy astropy deleted a comment from astropy-bot Bot Jul 8, 2019
@astropy astropy deleted a comment from astropy-bot Bot Jul 8, 2019
@pllim

pllim commented Jul 8, 2019

Copy link
Copy Markdown
Member

I deleted all the bot comments about closing the PR and see if the bot would behave properly and issue a "gonna close this" warning again next time it kicks off.

@astropy-bot

astropy-bot Bot commented Jul 9, 2019

Copy link
Copy Markdown

Hi humans 👋 - this pull request hasn't had any new commits for approximately 1 year, 7 months. I plan to close this in a month if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock.

If you believe I commented on this pull request incorrectly, please report this here.

@adrn

adrn commented Jul 14, 2019

Copy link
Copy Markdown
Member Author

I would like it to be on the table for 4.0! But I'm not sure where we are with this...I guess @mhvk and I are supposed to discuss?

@bsipocz bsipocz added this to the v4.0 milestone Jul 14, 2019
@astropy-bot

astropy-bot Bot commented Aug 10, 2019

Copy link
Copy Markdown

I'm going to close this pull request as per my previous message. If you think what is being added/fixed here is still important, please remember to open an issue to keep track of it. Thanks!

If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here.

@astropy-bot astropy-bot Bot added the closed-by-bot Closed by stale bot label Aug 10, 2019
@astropy-bot astropy-bot Bot closed this Aug 10, 2019
@pllim pllim mentioned this pull request Aug 12, 2019
Closed
2 tasks done
@nstarman

nstarman commented Aug 20, 2020

Copy link
Copy Markdown
Member

Having the unit system as a ScienceState and everything, even constants, derive their unit representation from the choice of unit system would be fantastic.
I recently ran into something like this in SymPy which has a cool implementation of unit and dimension systems.

Also, it would be amazing to have a symbolic version of units, constants, and quantities for numerical - symbolic interoperability.

@adrn adrn mentioned this pull request Apr 12, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhvk mhvk Awaiting requested review from mhvk
@eteq eteq Awaiting requested review from eteq

Assignees

No one assigned

Labels

closed-by-bot Closed by stale bot Experimental units

Projects

None yet

Milestone

v4.0

Development

Successfully merging this pull request may close these issues.

Add "extragalactic" and "galactic" unit systems

7 participants

@adrn @mhvk @eteq @astrofrog @bsipocz @pllim @nstarman