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

Should the Quantity constructor always return a Quantity? #584

Open
sostock opened this issue Nov 14, 2022 · 3 comments
Open

Should the Quantity constructor always return a Quantity? #584

sostock opened this issue Nov 14, 2022 · 3 comments

Comments

@sostock
Copy link
Collaborator

sostock commented Nov 14, 2022

Currently, Quantity(x, NoUnits) just returns x. One needs to call the constructor with type parameters to actually get a quantity:

julia> Quantity(3, NoUnits) |> typeof
Int64

julia> Quantity{Int, NoDims, typeof(NoUnits)}(3) |> typeof
Quantity{Int64, NoDims, Unitful.FreeUnits{(), NoDims, nothing}}

I would argue that the Quantity constructor should always return a Quantity (unless it errors, of course). There is a proposal for Julia 2.0 to enforce this for all constructors: JuliaLang/julia#42372.

@devmotion
Copy link

devmotion commented Apr 10, 2024

IMO the current behaviour of the Quantity constructor is a bug. I just ran into an issue caused by this in a project: In this project, values and units are received separately as floating point numbers and strings, respectively, and then parsed to a unitful quantity with

value * uparse(units)

However, due to the behaviour of Quantity we have

julia> typeof(1.0 * uparse("mg/dL"))
Quantity{Float64, 𝐌 𝐋⁻³, Unitful.FreeUnits{(mg, dL⁻¹), 𝐌 𝐋⁻³, nothing}}

julia> typeof(1.0 * uparse("L/L"))
Float64

The return type of "L/L" (units of hematocrit in the project) meant that suddenly the parsed server responses were incompatible with downstream structs that require an AbstractQuantity.

@sostock
Copy link
Collaborator Author

sostock commented Apr 15, 2024

IMO the current behaviour of the Quantity constructor is a bug.

I wouldn’t call it a bug (the behavior is intentional) and I certainly don’t think we should change it in a patch release. I think of this as a breaking change, something to do for Unitful v2.0.

julia> typeof(1.0 * uparse("L/L"))
Float64

Note that, even if it would return a Quantity, its units would be NoUnits, as there is currently no way to keep L/L as a unit.

@devmotion
Copy link

I think it's a bug because the Quantity constructor is "lying" about its return type. If I call Quantity(x) I expect that the return value is of type Quantity, which is a common expectatiob in Julia (IIRC there were discussions about enforcing it in Julia 2.0 but I'm not sure how likely such a change would be).

Note that, even if it would return a Quantity, its units would be NoUnits, as there is currently no way to keep L/L as a unit.

This is exactly what I think it should return and what my downstream applications would require - a Quantity with NoUnits.

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