Make u_str work with precompilation

[c42f]

I was a little disheartened to see that the (hugely convenient and awesome) u_str is not recommended for use with precompilation when used with unit plugin modules. I think the following approach fixes that: I've just added a check that the desired unit extension module is explicitly loaded into the module before the u_str macro is invoked.

Demo:

julia> using UnitfulUS

julia> module A
           using Unitful
           foo() = 1u"gal_us"
       end
ERROR: LoadError: Symbol `gal_us` was found in unit module UnitfulUS, but was not loaded into Main.A. Consider `using UnitfulUS` within `Main.A`?
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] replace_value(::Module, ::Symbol) at /home/tcfoster/.julia/dev/Unitful/src/user.jl:540
 [3] @u_str(::LineNumberNode, ::Module, ::Any) at /home/tcfoster/.julia/dev/Unitful/src/user.jl:495
in expression starting at REPL[2]:3

julia> module A
           using Unitful, UnitfulUS
           foo() = 1u"gal_us"
       end
WARNING: replacing module A.
Main.A

julia> A.foo()
1 galᵘˢ

[coveralls]

Coverage increased (+4.0%) to 78.609% when pulling 7724fd4 on c42f:cjf/precompilable-u_str into 2a972a0 on ajkeller34:master.

[codecov]

Codecov Report

Merging #201 into master will increase coverage by 4.17%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #201      +/-   ##
==========================================
+ Coverage   74.76%   78.94%   +4.17%     
==========================================
  Files          14       14              
  Lines         947      964      +17     
==========================================
+ Hits          708      761      +53     
+ Misses        239      203      -36

Impacted Files	Coverage Δ
src/user.jl	87.5% <100%> (+1.54%)	⬆️
src/conversion.jl	83.63% <0%> (+2.15%)	⬆️
src/logarithm.jl	69.37% <0%> (+2.22%)	⬆️
src/quantities.jl	91.56% <0%> (+3.68%)	⬆️
src/range.jl	78.94% <0%> (+5.26%)	⬆️
src/promotion.jl	33.33% <0%> (+8.33%)	⬆️
src/units.jl	88.33% <0%> (+9.34%)	⬆️
src/display.jl	90.62% <0%> (+9.37%)	⬆️
src/dimensions.jl	94.44% <0%> (+11.11%)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a972a0...7724fd4. Read the comment docs.

[codecov]

Codecov Report

Merging #201 into master will increase coverage by 4.17%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #201      +/-   ##
==========================================
+ Coverage   74.76%   78.94%   +4.17%     
==========================================
  Files          14       14              
  Lines         947      964      +17     
==========================================
+ Hits          708      761      +53     
+ Misses        239      203      -36

Impacted Files	Coverage Δ
src/user.jl	87.5% <100%> (+1.54%)	⬆️
src/conversion.jl	83.63% <0%> (+2.15%)	⬆️
src/logarithm.jl	69.37% <0%> (+2.22%)	⬆️
src/quantities.jl	91.56% <0%> (+3.68%)	⬆️
src/range.jl	78.94% <0%> (+5.26%)	⬆️
src/promotion.jl	33.33% <0%> (+8.33%)	⬆️
src/units.jl	88.33% <0%> (+9.34%)	⬆️
src/display.jl	90.62% <0%> (+9.37%)	⬆️
src/dimensions.jl	94.44% <0%> (+11.11%)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a972a0...7724fd4. Read the comment docs.

[c42f]

Updated tests to fix a difference in the exception type thrown in 0.7 vs 1.0.

[ajkeller34]

Thanks for looking at this. It seems like it should work, so after the tests pass I'll merge and we can see if there's any unexpected fallout :)

[ajkeller34]

This seems to have broken a few packages. Closer to the end of the week I hope to get a chance to look closer but it looks like your error message is popping up frequently, so some packages must not be doing what you think they should.

[c42f]

Hmm, darn. Nothing like trying it out to find your assumptions are a little broken... or alternatively that your implementation has become part of your public API against your best intentions. There's a bit of both here I think ;-)

The UnitfulAtomic error is due to having the unit definitions in a submodule - they actually come from UnitfulAtomic.RegisteredUnits rather than UnitfulAtomic itself. But the tests only do using UnitfulAtomic, and quite reasonably so. I think we could patch up the current approach in Unitful to also allow unit definitions in submodules of using'd modules. Fixing upstream is also possible though, what do you think @sostock?

The PowerSystemsUnits errors are due to the tests doing using Unitful: @u_str, so the name Unitful isn't actually in the namespace at all. Easily patched up in PowerSystemUnits itself.

The PowerModelsAnnex failure is due to the fact that PowerSystemsUnits doesn't register itself in its __init__, and PowerModelsAnnex does it instead as a workaround, but not in the tests... so I don't know how this ever worked properly. Quick workaround would be to manually register PowerSystemsUnits in the PowerModelsAnnex tests, CC @nicoleepp. There's a crossref to #193 in the PowerSystemsUnits source.

Can't tell what's wrong with Modea at a quick glance... looks like they're duplicating @u_str with their own @U_str, perhaps partly for the reasons this PR was trying to fix. So perhaps this change would help them to not do that anymore (or maybe #161 has fixed it already). @tshort any thoughts?

[sostock]

Fixing UnitfulAtomic is simple: The reason for using the submodule UnitfulAtomic.RegisteredUnits is that not all units defined in UnitfulAtomic are registered, because their names clash with those in Unitful. Simply changing the names of the units in UnitfulAtomic (e.g. ħ → ħ_au) would make it possible to register all units and thus get rid of the submodule. I should probably have done that in the first place, since having both Unitful.ħ (a quantity) and UnitfulAtomic.ħ (a unit) is kind of confusing. I will update my package accordingly.

Also, I think it would be good to mention this new restriction on @u_str usage somewhere in the documentation (even though the error message is pretty clear).

[sostock]

I just tagged UnitfulAtomic 0.2.0 (which is compatible with the new @u_str) and restricted UnitfulAtomic 0.1.0 to Unitful ≤ 0.13.0.