TheELNConsortium / TheELNFileFormat

Specification for the ELN File Format
MIT License
45 stars 9 forks source link

update eLabFTW examples #72

Closed NicolasCARPi closed 3 months ago

NicolasCARPi commented 3 months ago

still a WIP but I wanted to move forward before our meeting tomorrow!

jmanideep commented 3 months ago

Hi @NicolasCARPi,

A few remarks from my side:

NicolasCARPi commented 3 months ago

Hello @jmanideep and thank you for taking the time to review this change.

In the second node(where "@id": "./"), @type should be array of Dataset i.,e ["Dataset"].

Fixed.

sdPublisher and instrument nodes should be flattened.

Done.

We agreed to add properties only when they have non-null values. For instance, in this case, mentions, keywords, comment, and hasPart have empty arrays as values, variableMeasured has null, and text, alternateName have empty strings as values.

Empty values should not appear anymore now.

The Dataset type nodes have a category property added, although the Dataset itself does not have this property, similarly for the status property.

My mistake. So for status I've used creativeWorkStatus, and for category, I've used the about property, that points to a Thing node with the name of the category. I've also updated the correspondance table in the README, with everything that has been added lately.

@id property in Dataset nodes must end with /.

Fixed.

@type is missing in PropertyValue nodes.

Fixed.

Should empty values for the value property be included in PropertyValue nodes?

I'd argue that in this context of "extra fields", having no value is still information, and we still want to keep track of the field even if it's empty at the moment of export.

FlorianRhiem commented 3 months ago

The import of export.eln fails in SampleDB, as duplicate @id values a used. Checking the file I can see that #category-Project CRYPTO-COOL is defined multiple times. As the nodes for it contain the same information, I think this is just an issue with the about fields of Datasets containing actual values instead of references to nodes from the graph.

jmanideep commented 3 months ago

I'd argue that in this context of "extra fields", having no value is still information, and we still want to keep track of the field even if it's empty at the moment of export.

Handling these cases, especially those where "value": null, during the import is challenging because we currently rely on the value to infer the type of the value.

NicolasCARPi commented 3 months ago

@FlorianRhiem thank you for trying it out. It was the freshly added Category nodes that were added regardless if an existing one existed already. This is now fixed!

@jmanideep

we currently rely on the value to infer the type of the value

well, null is a type like any other ;)

FlorianRhiem commented 3 months ago

It now fails due to two comments (the one about the sky being blue and the one about water being wet) both having the same (therefore duplicate) ID.

NicolasCARPi commented 3 months ago

@FlorianRhiem thank you, this has been fixed.

NicolasCARPi commented 3 months ago

Oh yeah right, the file path of attachements in the ro-crate is different from the name of the file in zip directory! I've fixed that now.