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

Conversion fails to strip variable #697

Open
twildi opened this issue Nov 13, 2023 · 6 comments
Open

Conversion fails to strip variable #697

twildi opened this issue Nov 13, 2023 · 6 comments

Comments

@twildi
Copy link

twildi commented Nov 13, 2023

Running this snippet

using PhysicalConstants.CODATA2018: c_0

index = 1.858
c::Float64 = c_0 / index

raises

ERROR: DimensionError: and m s^-1 are not dimensionally compatible.

It is my understanding that this comes from the fact that Float64(c_0) returns a Unitful.Quantity{Float64, 𝐋 𝐓 ^-1, Unitful.FreeUnits{(m, s^-1), 𝐋 𝐓 ^-1, nothing}} instead of a Float64.

I don't think this should be the expected behavior and in such a case the variable should be stripped of its unit.

@giordano
Copy link
Collaborator

giordano commented Nov 13, 2023

If you want to strip units you have to do that explicitly with ustrip. If anything, I believe Float64(::AbstractQuantity) should throw an error instead of returning an AbstractQuantity.

@twildi
Copy link
Author

twildi commented Nov 13, 2023

That doesn't seem to match the expected behavior described here:

https://docs.julialang.org/en/v1/manual/conversion-and-promotion/

One would expect at least that convert(Float64, 1u"m") returns a Float64 and Float64(1u"m") as well unless there is good reason not to.

@giordano
Copy link
Collaborator

That doesn't seem to match the expected behavior described here:

That what? I suggested Float64(::AbstractQuantity) should error, which would agree with the documentation you're linking.

@twildi
Copy link
Author

twildi commented Nov 13, 2023

I quote :

Constructors that don't return instances of their own type

In very rare cases it might make sense for the constructor T(x) to return an object not of type T. This could happen if a wrapper type is its own inverse (e.g. Flip(Flip(x)) === x), or to support an old calling syntax for backwards compatibility when a library is restructured. But convert(T, x) should always return a value of type T."

@giordano
Copy link
Collaborator

I'm missing what you're trying to say:

julia> using Unitful

julia> convert(Float64, 1u"m")
ERROR: DimensionError:  and m are not dimensionally compatible.

convert(Float64, 1u"m") throws an error, because the conversion doesn't make sense.

Again, if you want to get a dimensionless number out of a number with units, use ustrip.

@twildi
Copy link
Author

twildi commented Nov 13, 2023

I think that most people will run into this issue in Packages that have Unitful.jl as a dependency, such as PhysicalConstants.jl : they would expect the constants from the package to be drop-in replacements for whatever hardcoded constants they have in their code, but they run into this behavior which brakes the abstraction.

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

No branches or pull requests

2 participants