DM-14998: Fix documentation on Schema field name conventions. #368
Conversation
doc/table.dox
Outdated
| * Other field strings (documentation and units) are essentially arbitrary, but should not contain | ||
| * single quotes, as these may also confuse FITS parsers (even when escaped). | ||
| * | ||
| * By convention, field names are CamelCase and have '_'-separated elements. |
There was a problem hiding this comment.
Would it be more accurate to say (c)CamelCase (or some other way of emphasizing that the initial letter need not always be capitalized)?
Might it help to provide a list of examples that fall into the category of "element" in this case (no worries if you think the meaning of the word is clear/unambiguous in the context of code)?
There was a problem hiding this comment.
I'll just write "camel case" here and not imply anything with how its capitalized. I think later uses are okay because they're right above the section saying that starting with lowercase is okay.
I think I prefer having the list of element examples in the next paragraph, just to keep this paragraph simple.
doc/table.dox
Outdated
| * single quotes, as these may also confuse FITS parsers (even when escaped). | ||
| * | ||
| * By convention, field names are CamelCase and have '_'-separated elements. | ||
| * Only letters, numbers and underscores should be used. |
There was a problem hiding this comment.
Comma after "numbers" (to be consistent with use below at line 171)?
include/lsst/afw/table/Schema.h
Outdated
| @@ -323,7 +325,7 @@ class Schema { | |||
| /** | |||
| * A proxy type for name lookups in a Schema. | |||
| * | |||
| * Elements of schema names are assumed to be separated by periods ("a.b.c.d"); | |||
| * Elements of schema names are assumed to be separated by underscores ("a.b.c"); | |||
include/lsst/afw/table/Schema.h
Outdated
| @@ -132,7 +134,7 @@ class Schema { | |||
| * Return a set of field names in the schema. | |||
| * | |||
| * If topOnly==true, return a unique list of only the part | |||
| * of the names before the first period. For example, | |||
| * of the names before the first underscore. For example, | |||
There was a problem hiding this comment.
Examples below ['a.b.c', 'a.d', 'e.f'] -> ['a_b_c', 'a_d', 'e_f']
| * selecting a prefix that is both concise and unambiguous. | ||
| * | ||
| * Note that some underscore-separated elements are themselves multiple words, | ||
| * such as `SdssShape` or `nChild`, and we use CamelCase, not more underscores, |
There was a problem hiding this comment.
I think I'll leave this one and the next as is; I don't want the punctuation to be confusing, either.
doc/table.dox
Outdated
| * such as `SdssShape` or `nChild`, and we use CamelCase, not more underscores, | ||
| * to separate words. | ||
| * A good rule of thumb is that if two words are not individually meaningful | ||
| * (or mean something different when separated), join them with CamelCase. |
doc/table.dox
Outdated
| * should start with uppercase (to match the name of the class). | ||
| * | ||
| * Schema provides extra functionality for names with underscore-separated | ||
| * elements; these elements can be accessed separately individually with the |
There was a problem hiding this comment.
"separately individually" -> redundant?
doc/table.dox
Outdated
| * | ||
| * Schema provides extra functionality for names with underscore-separated | ||
| * elements; these elements can be accessed separately individually with the | ||
| * bracket operators. More information on this behavior be found in the |
There was a problem hiding this comment.
"...behavior can be found..."
| * Field names in Schemas are expected to be dot-separated names (e.g. 'a.b.c'). The SubSchema | ||
| * Field names in Schemas are expected to be underscore-separated names (e.g. 'a_b_c', | ||
| * but see @ref afwTableFieldNames for the full conventions, including when to use | ||
| * underscores vs. CamelCase). The SubSchema |
TallJimbo
force-pushed
the
tickets/DM-14998
branch
2 times, most recently
from
01247a2
to
fdaf946
Compare
| * it. There are no hard rules for how to abbreviate the name of a Task | ||
| * when generating its field names; we trust developer judgement in | ||
| * selecting a prefix that is both concise and unambiguous. | ||
| * |
There was a problem hiding this comment.
Actually, could I ask that you include an example from one of the cases that pointed this issue out (just to be overly complete!)?
There was a problem hiding this comment.
Which cases do you mean? The ones on DM-14997? I'm vaguely aware that different Tasks do different things, which is why I've avoiding trying to declare a rule that I know we don't follow, but I haven't actually thought about which ones use good names and why.
There was a problem hiding this comment.
Yeah, like the calib_photometryUsed -> calib_photometry_used, for example. A call has to be made there to homogenize (or do you think that requires an RFC. I liked your argument...and I'm thinking the more examples we explicitly include here, the more likely it is that we will achieve consistency throughout).
There was a problem hiding this comment.
I think there will need to be an RFC regardless, because changing the schemas produced by the pipeline breaks any code that reads those outputs; it's like an API change.
I actually think the reason to prefer calib_photometry_used over calib_photometryUsed (now that I think about it) is that you want underscores when you want the prefix to be considered a conceptual group. I'll go ahead and add that rule of thumb as well.
There was a problem hiding this comment.
Ok, shall I write an RFC for which DM-14997 will be the implementation ticket?
There was a problem hiding this comment.
Ok, shall I write an RFC for which DM-14997 will be the implementation ticket?
Yup, that sounds perfect.
These docs should have been updated back in S15, when the code itself changed.
TallJimbo
force-pushed
the
tickets/DM-14998
branch
from
fdaf946
to
ed2d468
Compare
No description provided.