search

FAIRmat-NFDI / pynxtools

https://fairmat-nfdi.github.io/pynxtools/
Apache License 2.0
12 stars 8 forks source link

Always add definitions version #221

Closed domna closed 2 months ago

domna commented 5 months ago

Currently, we add a version string function but actually writing the nexus version has to be done by each reader. We should add a general way to add the definition and its version to a file independent of the reader.

This is how it's done for ellipsometry: https://github.com/FAIRmat-NFDI/pynxtools/blob/ac19484528443129ae947b492141c15918395365/pynxtools/dataconverter/readers/ellips/reader.py#L467-L472

This should be agnostic of the reader and should be done for every entry.

sherjeelshabih commented 5 months ago

We are not locking down the NXDL version with the reader here, right? We'll just provide a convenient function for readers to use to get the current definition version being used. If that is the case, why shouldn't we just use the conversion routine to verify it passes with the current version being used and just add it there? It means less code in the reader that has to be updated if we change the versioning function, etc.

domna commented 5 months ago

We are not locking down the NXDL version with the reader here, right? We'll just provide a convenient function for readers to use to get the current definition version being used. If that is the case, why shouldn't we just use the conversion routine to verify it passes with the current version being used and just add it there? It means less code in the reader that has to be updated if we change the versioning function, etc.

Yes, exactly. This was my idea with this issue. As long we use pynxtools the reader should not actually have to care about writing the definition being used and its version.

sherjeelshabih commented 5 months ago

Then your point is to allow reader's to have the freedom to place this arbitary version in any spot in the template they like. I can see why this helps.

What do you think about having readers not fill the version but just do it auto, like we do with a warning rn, in the converter? If the version will always be the one used by dataconverter, then we're good. I don't want to add a burden of responsibility for people dealing with reader's for something trivial.