Unit construction macro now defines docs for units

[lukebemish]

This is a fairly minor change that simply causes Docs.getdocs to be defined for the appropriate type when a unit is defined, allowing for basic documentation of the dimensions and conversion factor of every unit defined by the "unit" macro.

Close #436

[sostock]

Thanks! I think the docstrings could be improved:

help?> Unitful.m
"Equal to 10^0 Meter, or 1 m, with dimensions 𝐋"

help?> Unitful.mm
"Equal to 10^-3 Meter, or 1//1000 m, with dimensions 𝐋"

• The docstrings should probably mention that the object is a unit (not a quantity), because to me, “equal to 1 m” sounds like it describes a quantity.
• In the Unitful.m docstring, I don’t think it’s helpful to mention that m is equal to 1m.
• The quotes should not be included in the docstrings. Adding getdoc methods is not the right way to implement this, it’s better to add the docstrings in the expression returned by the macro. EDIT: or use Docs.doc!

Also, should this be optional? Maybe users want to write their own docstrings for units instead. We could add a new optional argument to the @unit macro that determines whether the automatic docstring is added.

I also think we should enable documenting the @unit macro by just adding Base.@__doc__ in the right place:

julia> "docstring for MySecond"
       @unit mys "mys" MySecond 1u"s" true
ERROR: cannot document the following expression:

#= REPL[20]:2 =# @unit mys "mys" MySecond 1 * u"s" true

'@unit' not documentable. See 'Base.@__doc__' docs for details.

However, this would document only mys, not mmys (milli-MySecond). Maybe the docstrings for the prefixed units should be added automatically even if one supplies their own docstring?

[lukebemish]

Alright; I agree with what you've said, though I'll have to do some research, as I haven't played with julia documentation too much. What do you think the docstring for the base unit should be (i.e., meters)? The reason I had implemented this using getdoc is that I couldn't find any easy way to come up with the appropriate doc string when the macro is run, but I'll look into better alternatives, as I agree that using getdoc is by far not the best option.

Allowing docstrings for base units should be fairly easy; I may go down that method instead. I could also probably implement a fairly simple addition for power-of-ten units; something like "A unit equal to 10^-3 meters" or the like.

[lukebemish]

I just added a couple of commits that should provide documentation in a completely different way, relying on user-defined documentation instead and only automatically generating the SI power-of-ten derived units documentation. All in all, I think this is a much nicer way to go, as users can add as much custom documentation as they would like to to any units they define. I also started adding documentation for some of the units defined in pkgdefaults, though I figured I should hold off on defining documentation for all of them until we've got a common documentation format. The system I'm using should also let documentation be defined for dimensions, if I did everything correctly. What do you think?

[codecov-commenter]

Codecov Report

Merging #476 (4787813) into master (5f68b2b) will increase coverage by 0.85%.
The diff coverage is 96.29%.

@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
+ Coverage   83.77%   84.63%   +0.85%     
==========================================
  Files          16       16              
  Lines        1344     1419      +75     
==========================================
+ Hits         1126     1201      +75     
  Misses        218      218              

Impacted Files	Coverage Δ
src/pkgdefaults.jl	18.60% <ø> (ø)
src/types.jl	89.79% <ø> (ø)
src/user.jl	95.05% <96.29%> (+0.78%)	⬆️
src/dates.jl	95.83% <0.00%> (+0.05%)	⬆️
src/logarithm.jl	69.67% <0.00%> (+0.12%)	⬆️
src/conversion.jl	88.40% <0.00%> (+0.17%)	⬆️
src/display.jl	95.45% <0.00%> (+0.21%)	⬆️
src/dimensions.jl	95.23% <0.00%> (+0.23%)	⬆️
src/units.jl	86.00% <0.00%> (+0.48%)	⬆️
src/quantities.jl	94.64% <0.00%> (+0.49%)	⬆️
... and 2 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 5f68b2b...4787813. Read the comment docs.

[lukebemish]

This has stagnated slightly, mostly because I am not sure if I have enough expertise to write a doc string for all the built-in units, and I am not yet sure what a standard doc string format for these units should be. I've given it a try for some simpler units, but I would love to see what somebody else thinks of this.

[sostock]

I like that the new iteration of this PR now enables the user to write their own docstrings, even if it means that we have to document every unit in pkgdefaults.jl by hand. I haven’t looked at the code in detail, hopefully I have time for it soon.

Regarding the format of the docstrings, I think something like the following would be nice. In particular I think it would be good to spell out the names of the units (meter, joule etc.):

    Unitful.m

The meter, base SI unit of length.

Dimension: [`Unitful.𝐋`](@ref).

    Unitful.J

The joule, an SI unit of energy, equal to 1 kg*m^2*s^-2.

Dimension: `𝐌*𝐋^2*𝐓^-2`.

    Unitful.u

The unified atomic mass unit (also called dalton), a unit of mass, equal to 1.66053906660(50)*10^-27 kg.

Dimension: [`Unitful.𝐌`](@ref)

The automatically generated docstrings for prefixed units could look like this:

    Unitful.cm

A prefixed unit, equal to 10^-2 m.

Dimension: [`Unitful.𝐋`](@ref).

See also: [Unitful.m](@ref).

For gram and its prefixed versions, one could add a disclaimer:

    Unitful.g

The gram, an SI unit of mass, equal to 10^-3 kg. Note that `kg`, not `g`, is the base unit.

Dimension: [`Unitful.𝐌`](@ref).

See also: [`Unitful.kg`](@ref).

[lukebemish]

Getting the dimension to show up in the generated docs for prefixed units is quite hard; I haven't yet found a method that works. In fact, I now realize that the way I was making a reference to the base unit was also flawed, but I'm fixing that now. If anyone has ideas for getting nice representations of dimension in prefixed units, that would be good to look at. Otherwise, I'll go with what I have now, which is to give the prefixed unit a reference to the base unit, and the base unit information on the dimension.

[lukebemish]

I've added documentation in this format for all units defined in pkgdefaults. It could definitely use some review; additionally, I'd like to figure out a standard format for log scales and log scale units, and I haven't really used those much at all in Unitful.

[sostock]

I suggest using the “official” SI names for the dimensions. I will look at the other docstrings later.

[sostock]

This batch of suggestions changes “base SI unit” to “SI base unit” and uses the official names for the base dimensions. It also changes “mol” to “mole” (“mol” is the symbol, “mole” the full name of the unit).

[sostock]

Some corrections to the docstrings.

[lukebemish]

All of these changes make sense; I believe I have committed all of them.

[sostock]

The suggestions below add dimension information to dimensionless units and explain the choice of symbols for q and minute.

Some more points about the format of the docstrings:

• The symbols that are documented should appear in the docstrings, i.e.,

  "A dimension representing length."
  

  should become

  "    Unitful.𝐋
  \nA dimension representing length."
  

• The \n\n at the beginning of some lines is unnecessary. Since there is already a linebreak preceding these lines, a single \n is enough to get a double linebreak.
• The format used for numbers and quantities in docstrings is inconsistent. Sometimes there is a space between number and unit (100 kPa), sometimes there isn’t (24hr). Sometimes, digits are grouped (299,792,458 m/s), sometimes they aren’t (6.62607015 e-34 J*s). In my opinion, if we want to keep the 6.62607015 e-34 J*s format, the space before e-34 should be removed (at least I have never seen it written with a space before). We should come to a conclusion what format we want (something that can be parsed or something that looks nice?).
• Some docstrings don’t give the actual values of the quantities:

   "A quantity representing the rest mass of an electron, given to 11 significant figures.
  

  This should include the actual value, not just a note that the value is given to 11 significant figures, because this information is rather useless without knowing which value is actually used. Better:

   "A quantity representing the rest mass of an electron, equal to 9.109,383,7015 × 10^-31 kg (the CODATA 2018 recommended value).
  

[lukebemish]

Several things to follow up on this:

• It might be worth using Dimension: [Unitful.NoDims](@ref) instead of Dimension: dimensionless for dimensionless units, as NoDims is what the dimension function would return from this value. What do you think of this?
• I like the following format for numbers in docstrings: 9.109,383,7015 × 10^-31. I think that this looks nice, and my argument against needing something parsable is that these are the docstrings for an object; if the parsable version is needed, that object can just be referenced. However, I would like to see other people's take on this.
• Additionally, I think that all values with more than a minimal number of digits should be written in scientific notation, though once again this could use other people's input.
• I am going to go with the "space between numbers and units" format going forward, as I think it looks more readable.

I'll put together a couple of commits for all of this shortly; the format questions I can change again if necessary if we decide on a different format.

[sostock]

These are just some cosmetic changes. Once we add tests, this should be good to merge.

[lukebemish]

What should we go with in the way of tests? I can't figure out any easy way to test if something is documented, which is the direction we'd probably like to go in.

[lukebemish]

Oh wait, docs can be cast to strings. I'll give this a try.

[sostock]

Oh wait, docs can be cast to strings. I'll give this a try.

Yeah, I would define a unit using the @unit macro and then check that string(@doc myunit) returns the correct string. And the same for the other documentable macros and autogenerated docstrings.

[lukebemish]

The tests are failing because multiline-string indentation is weird in julia 1.0:

help?> Unitful.kJ
                       Unitful.kJ

  A prefixed unit, equal to 10^3 J.

  Dimension: 𝐋^2 𝐌 𝐓^-2

  See also: Unitful.J.

I'm not really sure how to fix this, other than making sure no multiline text blobs are indented at all. This is a bit of a pain, but may be necessary for julia-1.0 compatibility. After some playing around with it, it looks like I might just have to remove the line break before code blocks.

[lukebemish]

I managed to fix this piece of the issue, but now I'm running into something else weird:

julia> string(@doc Unitful.Length)
"```\nUnitful.Length{T, U}\n```\n\nA supertype for quantities and levels of dimension [`Unitful.𝐋`](@ref) with a value                of type `T` and units `U`.\n\nSee also: [`Unitful.𝐋`](@ref), [`Unitful.Quantity`](@ref), [`Unitful.Level`](@ref).\n"

There's a random giant space in the middle. I think I know why; it's because that's where the indent is in the string. It disappears in the markdown, but unlike in Julia 1.6 doesn't disappear when I use string(@doc Unitful.Length). I think the only nice solution here is to just get rid of indents in multiline docstrings entirely, so that everything will be consistent between Julia versions.

[sostock]

What did you try to fix it? I think it should be enough to just replace every \n in triple-quoted strings by actual linebreaks:

    name_doc = """
                   $__module__.$name{T, U}

               A supertype for quantities and levels of dimension [`$__module__.$s`](@ref) with a value
               of type `T` and units `U`.

               See also: [`$__module__.$s`](@ref), $name_links.
               """

julia> string(@doc Unitful.Length)
"```\nUnitful.Length{T, U}\n```\n\nA supertype for quantities and levels of dimension [`Unitful.𝐋`](@ref) with a value of type `T` and units `U`.\n\nSee also: [`Unitful.𝐋`](@ref), [`Unitful.Quantity`](@ref), [`Unitful.Level`](@ref).\n"

[lukebemish]

I'll give that a try. That may be the issue.

[sostock]

I think this is finally ready to merge. Thank you for your work and for sticking with it so long!

[sostock]

I want to merge #489 first because it fixes the tests on master. Then we can re-run the tests on this PR and see whether they pass.

[sostock]

Apparently, re-running the jobs isn’t enough to get the tests to run on the merge commit into current master (they still use the older master without the fix). Maybe closing and re-opening the PR will do?