Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update eLabFTW examples #72

Merged
merged 7 commits into from
Jun 19, 2024
Merged

update eLabFTW examples #72

merged 7 commits into from
Jun 19, 2024

Conversation

NicolasCARPi
Copy link
Contributor

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

@jmanideep
Copy link
Contributor

Hi @NicolasCARPi,

A few remarks from my side:

  • In the second node(where "@id": "./"), @type should be array of Dataset i.,e ["Dataset"].
  • sdPublisher and instrument nodes should be flattened.
  • 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.
  • The Dataset type nodes have a category property added, although the Dataset itself does not have this property, similarly for the status property.
  • @id property in Dataset nodes must end with /.
  • @type is missing in PropertyValue nodes.
  • Should empty values for the value property be included in PropertyValue nodes?

@NicolasCARPi
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

@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
Copy link
Contributor

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
Copy link
Contributor Author

@FlorianRhiem thank you, this has been fixed.

Copy link
Contributor

@FlorianRhiem FlorianRhiem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can get it to work now, though there are some issues with the current SampleDB import, as the namelist for that zip file is returned with duplicate path separators, e.g. for ./RD - Testing-relationship-between-acceleration-and-gravity - 5adb0eb3/example.pptx the namelist lists it as ./RD - Testing-relationship-between-acceleration-and-gravity - 5adb0eb3//example.pptx so that SampleDB right now reports that file as not found. However the file can be imported successfully if I add a path normalization step, so I'll go with that. SampleDB doesn't import the variablesMeasured yet, I'm still working on that.

@NicolasCARPi
Copy link
Contributor Author

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.

@NicolasCARPi NicolasCARPi merged commit dc9c239 into master Jun 19, 2024
2 checks passed
@NicolasCARPi NicolasCARPi deleted the elab2024-06 branch June 19, 2024 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants