-
Notifications
You must be signed in to change notification settings - Fork 21
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
Finish writing QuantityPoint
reference
#152
Conversation
We take the `Quantity` reference as a template, and adjust the contents accordingly. Many sections can be omitted. For example, we don't support multiplication or division, and there is no "corresponding quantity point" machinery at this time. Other concepts are unique to `QuantityPoint`. For example, addition and subtraction always involve both `QuantityPoint` and `Quantity`. Also, there are further restrictions on implicit conversions in the case of units with offset origins that can't be represented as integers. A few other docs got updated as well. - Fixed a typo in the `Quantity` reference. - Added a link anchor in the `Unit` reference. - Added a section on quantity point makers in the unit slots discussion. To test this change, I clicked on every link and made sure it went where I was expecting. I also opened the `Quantity` and `QuantityPoint` reference docs in two neighboring tabs, and clicked on every corresponding section one at a time, tabbing back and forth to see the changes. Fixes #140.
Punctuation change -- semicolon to colon
Updated heading caps to sentence case
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.
Chip,
I had a couple questions/thoughts for you, especially on the quantity_point
file. Take a look and see if these things I've brought up make sense -- if you decide no changes are necessary, let me know and I'll approve.
John
### Quantity Point Maker (preferred) | ||
|
||
The preferred way to construct a `QuantityPoint` of a given unit is to use the _quantity point | ||
maker_ for that unit. This is a callable whose name is the plural form of that unit, expressed in |
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.
Is "callable" a noun, or an adjective in an unfinished phrase (e.g. a callable [something])
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.
Thanks for asking. I intended it to be a noun, which I think is a common choice.
docs/reference/quantity_point.md
Outdated
``` | ||
|
||
However, the _preferred_ way to construct a `QuantityPoint` is actually neither of these. It's the | ||
_quantity point maker_, which we describe next. |
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.
It's pretty confusing to introduce this concept by saying "There are two ways to construct a QuantityPoint
object (...description of ways...) but it's preferable not to use either one." -- the code block takes up a lot of space to demonstrate something that maybe shouldn't be the reader's first course of action.
So, notes:
- What do we accomplish by discussing these sub-ideal alternatives in such detail? Is it that the information is expected, or do either of these accomplish something unique that
Quantity Point Maker
doesn't/can't? - If these are necessary, is it possible to place them in a show/hide or an expand section, maybe something like:
Alternatives to Quantity Point Maker (header)
- Implicit Constructor from another
QuantityPoint
- Default constructor
- It's a little confusing that "Default constructor" is how we describe something that's not preferred. Is this a matter of convention, or have we assigned it that name/title/designation?
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.
Great callout, thanks. The reason this section reads as it does is because I copied it from the Quantity
doc and modified it. I've cleaned it up here, and also made similar improvements in the quantity doc. Specific cleanups include:
- Stating clearly up front that we prefer the quantity (point) makers.
- Splitting out the constructor signature expositions into their respective sections.
As for "default constructor", yes, that is the C++ term of art for a constructor that takes no arguments, and we don't expect our target users to find this confusing.
docs/reference/quantity_point.md
Outdated
the unit at the callsite. There are two functions which can do this, depending on whether you want | ||
to access by value or by reference. | ||
|
||
### `.in(unit)` {#extracting-with-in} |
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.
in the previous paragraph, you mention that the chosen function depends on whether the reader wants to access by value or by reference -- but the headers that follow make no mention of value or reference, that's left to the text.
Is it possible to place value or reference in the header instead? (I know that this information is in the first sentence following those headers and understand why this looks like a minor quibble, but it does help these sections to better parallel the introduction)
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.
Done: here, and in the quantity doc.
docs/reference/quantity_point.md
Outdated
|
||
??? example "Example: forcing a conversion from centimeters to meters" | ||
`centi(meters_pt)(200).as(meters_pt)` is not allowed. This conversion will divide the | ||
underlying value, `200`, by `100`. While this particular value would produce an integer result, |
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.
Can we use an example that wouldn't divide into an integer result evenly -- perhaps 250 cm as m? Using example values that would produce a valid int
result doesn't emphasize your point as strongly.
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 actually wanted a result that divides evenly! I expect human readers to notice that these specific numbers would work, but I wanted to remind them that the compiler can't know the actual number when it makes the permit/forbid decision, as the program is being built. Note that the third paragraph drives the point home by showing what would happen if we forced the conversion with an inexact multiple.
That said, I think the wording here could be much better, so I cleaned it up. I made my emphasis more explicit, and I combined the second and third paragraphs because they're both about forcing.
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.
Thanks for the great review! I like it much better after addressing your comments.
### Quantity Point Maker (preferred) | ||
|
||
The preferred way to construct a `QuantityPoint` of a given unit is to use the _quantity point | ||
maker_ for that unit. This is a callable whose name is the plural form of that unit, expressed in |
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.
Thanks for asking. I intended it to be a noun, which I think is a common choice.
docs/reference/quantity_point.md
Outdated
``` | ||
|
||
However, the _preferred_ way to construct a `QuantityPoint` is actually neither of these. It's the | ||
_quantity point maker_, which we describe next. |
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.
Great callout, thanks. The reason this section reads as it does is because I copied it from the Quantity
doc and modified it. I've cleaned it up here, and also made similar improvements in the quantity doc. Specific cleanups include:
- Stating clearly up front that we prefer the quantity (point) makers.
- Splitting out the constructor signature expositions into their respective sections.
As for "default constructor", yes, that is the C++ term of art for a constructor that takes no arguments, and we don't expect our target users to find this confusing.
docs/reference/quantity_point.md
Outdated
the unit at the callsite. There are two functions which can do this, depending on whether you want | ||
to access by value or by reference. | ||
|
||
### `.in(unit)` {#extracting-with-in} |
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.
Done: here, and in the quantity doc.
docs/reference/quantity_point.md
Outdated
|
||
??? example "Example: forcing a conversion from centimeters to meters" | ||
`centi(meters_pt)(200).as(meters_pt)` is not allowed. This conversion will divide the | ||
underlying value, `200`, by `100`. While this particular value would produce an integer result, |
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 actually wanted a result that divides evenly! I expect human readers to notice that these specific numbers would work, but I wanted to remind them that the compiler can't know the actual number when it makes the permit/forbid decision, as the program is being built. Note that the third paragraph drives the point home by showing what would happen if we forced the conversion with an inexact multiple.
That said, I think the wording here could be much better, so I cleaned it up. I made my emphasis more explicit, and I combined the second and third paragraphs because they're both about forcing.
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.
This looks good to me.
We take the
Quantity
reference as a template, and adjust the contentsaccordingly.
Many sections can be omitted. For example, we don't support
multiplication or division, and there is no "corresponding quantity
point" machinery at this time.
Other concepts are unique to
QuantityPoint
. For example, addition andsubtraction always involve both
QuantityPoint
andQuantity
. Also,there are further restrictions on implicit conversions in the case of
units with offset origins that can't be represented as integers.
A few other docs got updated as well.
Quantity
reference.Unit
reference.To test this change, I clicked on every link and made sure it went where
I was expecting.
I also opened the
Quantity
andQuantityPoint
reference docs in twoneighboring tabs, and clicked on every corresponding section one at a
time, tabbing back and forth to see the changes.
Fixes #140.