-
Notifications
You must be signed in to change notification settings - Fork 122
add dimensions docs to default units docs chapter #794
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
Conversation
docs/src/defaultunits.md
Outdated
| ``` | ||
| Unitful.Amount | ||
| ``` | ||
|
|
||
| Supertype for quantities and levels of dimension `Unitful.𝐍` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
| ``` | |
| Unitful.Amount | |
| ``` | |
| Supertype for quantities and levels of dimension `Unitful.𝐍` | |
| ```@docs | |
| Unitful.Amount | |
| ``` |
? Here and everywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@docs block apparently wouldn't generate documentation for non-exported objects. In any case in my tests I'm getting an error if trying it with Unitful.Amount or other internal object, whereas the following in the same place works as expected:
```
@docs; canonical=false
Unitful.Quantity
```
AI suggested some workarounds which may work or not work.
On the other side, the whole chapter had been generated by my script #704 from docstrings. Now dimensions were added, generated from docstrings as well. Thus I see no reason to treat them differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot confirm that docstrings for non-exported objects don’t get rendered. The problem with including only the Unitful.Amount docstring is that it contains a reference to Unitful.𝐍, so that docstring needs to be included as well. The following builds and renders fine for me:
```@docs
Unitful.Amount
Unitful.𝐍
```
I think that using @docs blocks has several advantages:
- Due to the rendering as blocks there is a stronger optical separation between the docstrings.
- Docstrings can be collapsed, which is nice on a page that contains so many of them.
- When building the docs there is a warning that lists all docstrings that are not included in the docs. That list currently contains all units, dimensions etc. Including them would make this list much shorter, so that it can actually be used to see what is still missing in the docs.
- Less code needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, this way it works, so I'd adapt the script and generate updated version of the docs chapter in the next days. Thank you.
(both Gemini 2.5Pro and Claude 3.5 told me "the @docs block requires that the documented symbols be exported..." 😕 - and as it indeed thrown error, there was a reason to beleive them)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both Gemini 2.5Pro and Claude 3.5 told me "the @docs block requires that the documented symbols be exported..."
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now updated, and docs build went without issues.
|
Documentation build uploaded as artifact at https://github.com/JuliaPhysics/Unitful.jl/actions/runs/17387815527/artifacts/3901204474, so that people don't need to build locally to see the output. |
| #### Yard | ||
|
|
||
| ``` | ||
| Unitful.nm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize that it’s ordered alphabetically, but I don’t like that nanometer is between mile and yard, and nowhere near meter. Generally, I’d prefer if the SI units are always the first in each section. But alphabetic order also makes sense.
I also don’t like that some prefixed units are included (because they are needed) and others are not. I would list all prefixed units after the corresponding non-prefixed unit, but in a collapsed section (like I suggested earlier). That could be added in a follow-up PR, it doesn’t need to happen now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd leave this for a follow up PR if possible 🙂
Suggested-by: Sebastian Stock <[email protected]>
This update primarily adds documentation on dimensions.
Other (minor) change: Physical constants now sorted ignoring case.
The PR is based on my another PR #704 , but can be merged independently.