PainterQubits / Unitful.jl

Physical quantities with arbitrary units
Other
604 stars 111 forks source link

Namespacing confusion: `uparse("myMeter")` example from docs gives ArgumentError #435

Open anandijain opened 3 years ago

anandijain commented 3 years ago
using Unitful
module MyUnits; using Unitful; @unit myMeter "m" MyMeter 1u"m" false; end
Unitful.register(MyUnits);
u"myMeter" # works

julia> uparse("myMeter")
ERROR: ArgumentError: Symbol `myMeter` was found in the globally registered unit module Main.MyUnits
but was not in the provided list of unit modules Unitful.

(Consider `using Main.MyUnits` in your module if you are using `@u_str`?)
Stacktrace:
 [1] lookup_units(unitmods::Vector{Module}, sym::Symbol)
   @ Unitful ~\.julia\packages\Unitful\PcVKX\src\user.jl:575
 [2] lookup_units
   @ ~\.julia\packages\Unitful\PcVKX\src\user.jl:593 [inlined]
 [3] uparse(str::String; unit_context::Module)
   @ Unitful ~\.julia\packages\Unitful\PcVKX\src\user.jl:536
 [4] uparse(str::String)
   @ Unitful ~\.julia\packages\Unitful\PcVKX\src\user.jl:535
 [5] top-level scope
   @ REPL[31]:1
using Main.MyUnits
uparse("myMeter") # still fails
using Unitful
Unitful.register(@__MODULE__);
@unit U "U" U 1u"μmol/60s" true
1u"U"
uparse("U") # fails 
ERROR: ArgumentError: Symbol `U` was found in the globally registered unit module Mainbut was not in the provided list of unit modules Unitful.

(Consider `using Main` in your module if you are using `@u_str`?)
using Main
uparse("U") # still fails

There may be a problem with uparse or I am not understanding namespacing. It is confusing to me why neither examples can be copy pasted to REPL

dmoored4 commented 3 years ago

I was having the same problem and was able to get it to work. Try changing to this:

uparse("MyMeter", unit_context=MyUnits)

I think this is okay if you have a totally different dimension, but could be troublesome if you have new units in the same dimension. For example, I have added "kips" which is used in engineering for 1000 lbf. If I'm trying to parse any string for Force units, that will be a problem because I won't know if I have to use the ctx or not at the outset.

Some other things I might investigate is importing the uparse function into my module and re-exporting it or maybe creating an outer function which will first try the regular units and if it fails then try my units? That might be the simplest option, though a little hacky.

Maybe this can spark some ideas for you. Let me know if you come up with anything good!

Update: this worked quicker than I expected:

module AugmentedUnits
export uparse
using Unitful

Unitful.register(@__Module__)

@unit    kip    "kip"    Kip    1000u"lbf"    false

function Unitful.uparse(str)
    try
        uparse(str, unit_context=Unitful)
    catch
        uparse(str, unit_context=AugmentedUnits)
    end
end

end

It's important to have the unit_context=Unitful in the first try block. Also, you'll need to parse the units separately. For my example, I would need to do:


unit_str_1 = "kip"
unit_str_2 = "ft"
unit_str_3 = "kip*ft"

1000*uparse(unit_str_1) * 1*uparse(unit_str_2)

At that point you should be able to convert and such as normal. But if you were to try:

1000*uparse(unit_str_3)

It will fail because in the try it will fail because it can't read kips and in the catch it will also fail because it doesn't know what ft are any more.

This works, but I don't necessarily endorse it as a good solution.

OmegaLambda1998 commented 2 years ago

Has there been any update to this, is there a better way of dealing with this issue?

dmoored4 commented 2 years ago

Just running into this issue again and I was able to understand a bit better. The suggestion in the error message did not solve the problem:

ArgumentError: Symbol `kip` was found in the globally registered unit module BlastedUnits
but was not in the provided list of unit modules Unitful. (Consider `using MyUnitsPackage`
in your module if you are using `@u_str`?)```

The try-catch in my previous post is unnecessary. Instead:

uparse("kip", unit_context=[Unitful, MyUnitsPackage])

I noticed that there is that in the Unitful.jl file there is const unitmodules = Vector{Module}().

If I call Unitful.unitmodules I get a vector of [Unitful, MyUnitsPackage] so I can even just write:

uparse("kip", unit_context=Unitful.unitmodules). 

This seems likely to be what most people would want and I'm wondering if this could be the default so that the kwarg unit_context isn't necessary. It seems like it would be very confusing and rare that you sometimes wanted to parse "m" as a meter from the Unitful defaults and sometimes from MyUnitsPackage.

Also maybe I'm using it in an unintended way by using @Reexport Unitful so that when I load MyUnitsPackage I don't also have to load Unitful.

Is there any reason to not have unit_context default to Unitful.unitmodules?

dmoored4 commented 2 years ago

Seems like a straightforward fix but I can't figure out how to contribute. In the mean time, this is working.

module MyUnitPackage
using Reexport
@reexport using Unitful
import Base.parse
parse(str::String) = uparse(str, unit_context=Unitful.unitmodules)

Unitful.register(@__MODULE__)
...

const localpromotion = Unitful.promotion

function __init__()
    Unitful.register(MyUnitPackage)
    merge!(Unitful.promotion, localpromotion)
end

end

Tbh I'm pretty confused about how this works. If I made uparse(str::String) = uparse(str, unit_context=Unitful.unitmodules) I get a compile error which makes sense because it has the same argument type as Unitful. But leaving it as an untyped argument allows me to do this.

For now I'll just be using parse("kip") because parse(str::String) is not already defined in base.