fNIRS / snirf

SNIRF Format Specification
http://fnirs.org/resources/software/snirf/
Other
59 stars 34 forks source link

How to address formatVersion for drafts #51

Closed rob-luke closed 3 years ago

rob-luke commented 3 years ago

I am writing code for reading and writing snirf files. And I would like to check against specific versions of the specification. Currently I am targeting draft 3 which has a ‘formatVersion’ of 1.0.

will draft 4 also have a ‘formatVersion’ version of 1.0? And if so, how is the software to differentiate these two versions? My understanding is that snirf has been officially released [1]. So it would be useful to track different versions in our continuous integration.

if there is currently no way to differentiate between files created with different drafts then can I suggest that we specify that files with v1 draft 3 have ‘formatVersion’ 1.0.3 to indicate a minor non breaking change. And draft 4 would be 1.0.4? Or something like https://semver.org/

sstucker commented 3 years ago

Unless I'm mistaken, unfortunately formatVersion was created but never really implemented.

Sidenote: as far as I can tell, no breaking changes have ever been made the snirf specification since files have been generated. The required fields especially. Because many changed or added fields are designated optional, your reading of the files should forgive most changes.

I am fine with implementing formatVersion going forward (starting with 1.0 as of today). Any changes made to this specification should increment this version value on this spec document.

I might be missing some history here? @fangq @jayd1860

dboas commented 3 years ago

It should be easy to implement. We just have to increment the version number each time a change to the spec is made and document the changes for each version. What is the best way to do that with github?

rob-luke commented 3 years ago

What is the best way to do that with github?

Probably using the releases feature in GitHub. When you make a new release it will also create a tag in git, which allows you to go back to specific points in time. It also allows you to compare the changes between releases.

See https://github.com/mne-tools/mne-nirs/releases for an example of releases.

There may be cleaner ways, this is the only way I've tried.

rob-luke commented 3 years ago

Sidenote: as far as I can tell, no breaking changes have ever been made the snirf specification since files have been generated

That is fantastic to know.

sstucker commented 3 years ago

I would love to start having titled releases. Would love to get input from @huppertt / @fangq / @jayd1860 in what we should title the first release, and if there is something critical missing before formally advancing to this "1.0". Otherwise will call the current spec v1.0 and everything prior will be "draft".

rob-luke commented 3 years ago

before formally advancing to this "1.0".

As an end user I have to admit to being a bit confused. On the GitHub page the document says draft 4. But on the sfnirs website (and other internet pages) it says that "The specification can be found on GitHub here. Version 1.0 was released in November of 2019." And as the field is mandatory I assume the vendors currently producing files already have the field set to 1.0. Due to this ambiguity I’d suggest you bump the version number above 1.0 for the next draft/release.

Regardless, I appreciate you considering this issue (and it’s probably largely academic as you haven’t broken backwards compatability). And moving forward I will implement whichever version number you settle on.

Thanks

dboas commented 3 years ago

@jayd1860 @fangq @sstucker , if Nov 2019 was version 1.0 and we had significant fixes in the Spring of 2020, it seems like that should have been version 1.1. What do you think?

sstucker commented 3 years ago

You are both right-- probably wrong to title anything forthcoming 1.0.

Should we settle on a definition for 1.1 in post? Or wait until new changes?

fangq commented 3 years ago

I suppose this was a confusion I created.

when we initially converted the drafted specs to Github, I labeled it as "Version 1 Draft X" - this means the targeting spec is Version 1.0, but we are still at the draft stage - I decided to use this format by following IETF-like internet standards, such as

https://json-schema.org/specification.html https://datatracker.ietf.org/doc/html/draft-bormann-cbor-09 https://datatracker.ietf.org/doc/html/draft-bormann-apparea-bpack-01

a new specification typically evolves through many drafts before solidifying as a version (stable release).

https://tools.ietf.org/id/draft-claise-semver-00.html

When they are in "work-in-progress", they are typically cited as draft-xxx-00/01/02/..., and some some project, simply draft-number. The understanding is that using a drafted specification indicates the fluid nature of the document, and the software shall anticipate some semantic/structural changes. However, once a standard becomes final - or at least obtain a formal version number, the freedom of making structural changes becomes much less, and the maintainers have to be extra careful not to break compatibility.

My thought is that this community is relatively small, and the spec is relatively young and untested, so, leaving it in the draft/work-in-progress stage could be beneficial - in the sense that we can still make changes like dropping/adding new fields or redefine data record formats.

On the other side, I would follow @dboas's decision on whether we should call the version released in Nov 2019 as Version 1, and any new additions should be referred to as draft 1/2/3/... leading to Version 2. If we do have an official V1 release, we have to make sure some of our tools/validators is V1 compatible without the later added features.

rob-luke commented 3 years ago

My thought is that this community is relatively small,

Small, but growing! This is a great initiative, and many people outside Boston are now jumping on it too. As such, its important to document things that may be obvious to the core developers.

On the other side, I would follow @dboas's decision on whether we should call the version released in Nov 2019 as Version 1

In that case, I would make the following suggestions:

  1. Note somewhere on the GitHub readme page where people can find the actual version 1 of the spec. It can just be a link to a specific git commit.
  2. Change the version number in the document to be Version 2 - Draft 3 to indicate draft 3 leading to version 2 (according to the IETF-like approach you describe above).
  3. I would suggest adding information about the versioning system used to the readme page. The IEFT-like approach you use is a standard, but not the only standard, so best to clarify to users.
    • Also you can note here that everything so far is backwards compatible, and that V2 just adds new features, does not change previous requirements.
  4. Use a git tag and the GitHub releases to mark what should be considered the actual version 1 of the spec.

Finally, I am here to help (not just cause trouble :wink:). Would you like me to open a PR to start on some of these suggestions?

fangq commented 3 years ago

Re: @rob-luke

Small, but growing! This is a great initiative, and many people outside Boston are now jumping on it too. As such, its important to document things that may be obvious to the core developers.

by "small" I actually meant in a positive way that we, the developers, still have the luxury to make some significant adjustments to the spec without worrying about these changes may break production codes.

Also you can note here that everything so far is backwards compatible, and that V2 just adds new features, does not change previous requirements.

not entirely. some of the changes were not backward compatible, see this Draft-4 worklist following a discussion we had back in Jan 2020

https://github.com/fNIRS/snirf/issues/44

Change the version number in the document to be Version 2 - Draft 3 to indicate draft 3 leading to version 2 (according to the IETF-like approach you describe above).

my initial treatment of the SNIRF format Version 1.0 is like the number 5 in the HTML5 standard - in the case of HTML5, there were at least 10 drafts leading to the "finished" version of the HTML5 specification

https://www.w3.org/TR/2011/WD-html5-20110405/

if there is no particular benefit of backtracking the 1.0 milestone version to an old (Nov 2019) and now-outdated version, perhaps I could recommend to refer the Nov.2019 version as Version 0 (like a preview) - let's complete the current outstanding items in the Issues, particular the Draft-4 work items in #44, and then we can call Draft-4 as the actual Version 1.0.

Moving forward, I can drop the "Draft X" notation, and use the 2nd digit in the formatVersion to label new milestone releases:

rob-luke commented 3 years ago

Thanks @fangq for clarifying this.

My only request is that this information is summarised somewhere accessible to end users (maybe also update the SfNIRS page to indicate that you are only at draft stage), people won't find it buried here in this issue. I think its clear from the discussion above that it is unclear how people should use the formatVersion field in SNIRF files, and that the versioning system used in this project is undocumented.

This is a small nitty gritty issue, but will help with those of us trying to write SNIRF readers and writers.

I appreciate the great work. Cheers, Rob

dboas commented 3 years ago

@fangq , companies and academics are now supporting SNIRF in their software. We do have to worry about not breaking production codes. From the above discussion, it is not clear to me what has been resolved about how to refer to version and modify version numbers in the future. I agree with @rob-luke that it would be great to have this clarified in the spec itself.

I am comfortable with this present version being called version 1.0 and no longer being referred to as a DRAFT.

Someone has to educate me then on what happens if we want to make minor adjustments that are backward compatible.

rob-luke commented 3 years ago

Just as a follow up. A few weeks back, MNE changed our code to store the string 1.0 - Draft 3 in the field formatVersion for our SNIRF writer based on our understanding of @fangq comments. If Davids suggestion to change the current version to 1.0 is accepted, then we will modify our code accordingly. Thanks, Rob

fangq commented 3 years ago

@dboas and @rob-luke, in this case, I suggest we use #44 to apply the final batch of fixes, and call the most up to date version, after checking off all #44 items, as Version 1.0.

I will also add a blurb in the spec to discuss how we manage version numbers.

formatVersion is defined as a string, so I assume most of us are ok with major.minor<.patch><-release> where major/minor and patch are integer numbers, and patch/release tags are optional.

rob-luke commented 3 years ago

I assume most of us are ok with

Happy with whatever scheme you choose. I just ask that its clearly specified so that I can use it as a validation check in the SNIRF reader, and so I know what to export in the SNIRF writer.

As an additional suggestion, could you provide static links in the README to these milestone versions you refer to (e.g. Nov.2019, 1.0 - Draft 3, 1.0)? Then when I get a file that has Nov.2019 in the formatVersion I can track down the spec at that point and know how to update my reader accordingly.

Someone has to educate me then on what happens if we want to make minor adjustments that are backward compatible.

@fangq seems to have a scheme in mind. But from the view of an end user, a changelog would help to know what was modified with each version.


As always, happy to help and contribute in any way. Just let me know what I can do to assist.

dboas commented 3 years ago

@fangq , your proposal works for me.

@rob-luke , where should we keep the change log? BTW, your help is much appreciated. What more can you do? Maybe set up the change log? Start this as a new issue?

rob-luke commented 3 years ago

I consider this closed with the merging of #58. The Nov 2019 version is not called 1.0 - Draft 3 and 1.0 should be release imminently. Thanks @dboas and @fangq, I am closing this issue now.