-
Notifications
You must be signed in to change notification settings - Fork 6
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
Using Unitful.jl & UnitfulAstro.jl for unit handling #48
Comments
Yes and no 😄 While I believe that having units available would be great, I also hold the opinion that they should not be the default. In my experience they make the implemention quite a bit more complicated and often require some nasty workarounds (the caveat here is that the only truly "unitful" code base I worked with was AstroPy and it has been a while since I last unsuccessfully tried to introduce Unitful.jl in AstroBase). My plan was to offer unit support as an extension on top of AstroBase.jl, e.g. UnitfulAstroBase.jl. This is the reason why constants in AstroBase are implemented as functions so that one can offer differently typed constants via multiple dispatch. For example: using Unitful
# One could overload `speed_of_light` like this
speed_of_light(::Type{<:Quantity}, unit::Unitful.VelocityUnits) = unit(SPEED_OF_LIGHT * u"m/s")
speed_of_light(T::Type{<:Quantity}) = speed_of_light(T, u"m/s") This is not set in stone, though 🤔 |
That makes sense, that's a great idea to keep constants as functions so they can be overloaded. The reason I asked is I'm developing an Astrodynamics library alongside a graduate Astrodynamics course this semester. The library is really just for me to learn about Astrodynamics, to help with my assignments, and to get some more practice with Julia. My library requires |
That would be great and certainly much appreciated 👍 |
FYI @cadojo: This issue here nicely illustrates why units by default might not be the best solution JuliaSpace/SatelliteToolbox.jl#44 |
AstroBase
currently uses its own unit types, and is limited to the set of units it implements. Is integratingUnitful
andUnitfulAstro
a desired feature? If so, I'd be interested in implementing this. But I wanted to check to see if that was a desired feature forAstroBase
first.The text was updated successfully, but these errors were encountered: