Restructure RFC documents to use a YAML front matter for Status and author/reviewer/commenter metadata - #552
Restructure RFC documents to use a YAML front matter for Status and author/reviewer/commenter metadata#552lubianat wants to merge 30 commits into
Conversation
Automated Review URLs |
|
Might benefit from a template, i.e. for RFCs and comments? |
|
Also; it may be worth to drop the affiliations from the rendered text, similar to how it's done at ngff-spec. The reasoning there was, that a person's orcid is likely informative enough about past and present affiliations. |
in theory, yes, it should. in practice, I need to keep the information provided by the authors on under which affiliation that comment was made. an example is the rfc 9 / comment 5 Anna Kreshuk - ilastik was the intended affiliation, though that is a project and not an employee, and not listed on https://orcid.org/0000-0003-1334-6388 |
Partially addresses ome#576
supported by Claude Opus 5
copying format of jbms in rfc 2
support by Claude Opus 5
|
Did you mean to bump |
gouttegd
left a comment
There was a problem hiding this comment.
Just commenting for now. I have mostly looked at the LinkML stuff, not the Python code in details.
| status_note: historical RFC, outdated by RFC-1 | ||
| description: Original consensus model for decision making | ||
| ome_zarr_version: "N/A" | ||
| date: 2024-08-30 |
There was a problem hiding this comment.
LinkML-defined, date-typed slots should always be written as strings in YAML serialisation. That is, they should be quoted (date: "2024-08-30").
Not quoting them will make the YAML parser parse the value into a date object, which may cause all sorts of unexpected problem down the line (see linkml/linkml#1532 for some details).
There was a problem hiding this comment.
OK, I see you have an explicit workaround in validate.py for that. I don’t like it, but well, it works. :)
| parts = text.split("---", 2) | ||
| if len(parts) < 3: | ||
| raise self.error("rfc-authors: no YAML front matter found") | ||
| meta = yaml.safe_load(parts[1]) or {} |
There was a problem hiding this comment.
Just so you know, since you have a LinkML schema, you could also use it to generate a RFCFrontMatter Python object to deal more easily with the contents of the front matter.
That is, you could produce a frontmatter.py Python module:
$ linkml generate pydantic front_matter.yaml > front matter.pyand then in this script:
from frontmatter import RFCFrontMatter
from linkml_runtime.loaders import yaml_loader
[...]
meta = yaml_loader.load(parts[1], RFCFrontMatter)The meta variable now contains a RFCFrontMatter object whose members you can use directly, instead of having to query the dictionary to get each key:
affils, order = {}, []
for author in meta.authors:
if author.affiliation and author.affiliation not in affils:
order.append(author.affiliation)
affils[author.affiliation] = len(order)And so on.
Not requesting any change because your way of course also works, but it does feel strange (to me at least!) to be using LinkML only halfway through like that. :)
There was a problem hiding this comment.
Oh, interesting! The LinkML schema came after document_authors.py, that is why the piecer are not connected, so a refactoring may make sense indeed.
| # resources/rfc-status-codes. The code in an RFC's front matter stays the source | ||
| # of truth; this only spells it out for readers. | ||
| STATE_LABELS = { | ||
| "D1": "Initial idea", |
There was a problem hiding this comment.
This is another thing that a LinkML-generated Python module could do for you. :)
| role: | ||
| description: >- | ||
| The person's role on this RFC, shown in the "Status" column, e.g. | ||
| "Corresponding Author", "Co-author", "Editor" or "Implemented". |
There was a problem hiding this comment.
Is that supposed to a free-text value? Or is there a clearly defined list of roles that people should pick from? If the latter, this could be an enum.
There was a problem hiding this comment.
I think free text for now, there is no strong "role" definition in the RFC process and maybe an enum is overspecifying
| description: The RFC's authors, in the order they should be credited. | ||
| range: Person | ||
| multivalued: true | ||
| inlined_as_list: true |
There was a problem hiding this comment.
Just FYI, this is not necessary: Person does not have either a “key” slot or an “identifier” slot, so it can only be inlined as a list, regardless of whether you say that explicitly or not.
(Though I do myself like to always make that explicit, as it makes the intention clearer. ;) )
| """Turn the dates PyYAML parses back into ISO strings. | ||
|
|
||
| An unquoted `date: 2025-07-02` becomes a datetime.date, while a quoted one | ||
| stays text; the pages treat both the same, so validation should too. |
There was a problem hiding this comment.
I would much rather consistently use quoted dates rather than having such a workaround…
That is excellent, thank you. I'll actually refactor some of it as you suggested. As I mentioned in the PR description, there is a lot that was generated with Claude assist. Reviewed, but still, it is a big change, so I am not sure how to make the review process less painful. |
|
Something that bumps me mildly is that the author information is rendered differently for the RFC document itself (the old table) and the reviews/comments/etc, where the frontmatter yaml is used. I think jupyter-book or myst-md are also able to do this based on frontmatter yaml that looks very similar to what we have here. If you check the auto-built metadata examples and schema markdown files that are generated by the to make sure that we don't render the author information on every page of the examples and schemas section ^^" |
First step towards a system with more automation/standardization for rfc status management.
German-BioImaging/ome-zarr-ideas#54
Next steps:
EDIT: Ready to review!
To review:
Compare the generated RFC pages with the older one.
Provide feedback on the new look and feel and if changes are needed
Compare the page listing the different RFCs with the older one
If checking all 57 documents is too much, pick a sample and see if it is okay
If you are a schema person, look at
rfc/schema/front_matter.yamlfor the LinkML schemaIf you are a sphinx person, look at
_extanddocument_authors.pyandrfc_status.pyand see if the directives make senseNote that a lot of the code and changes have been aided by a Claude Opus 5 agent, though I tried to steer for minimal changes and maximize readability.