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

Added format FID, FCF, HKL #820

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

cb2993
Copy link

@cb2993 cb2993 commented Mar 24, 2022

Hi,
As already known, @sneumann and I have looked into chemical data formats on behalf of @NFDI4Chem and want to add the format FID, FCF and HKL.

The issues #787 , #788 , #789 have already been created and herewith the corresponding PR shall be submitted. I have left out the unsafe parts for now.

Yours, Claudia.

(The already submitted format #796 is also included.)

@LucieLamothe
Copy link
Collaborator

Thank you so much for this PR it's good to tackle these issues! 👍

@LucieLamothe
Copy link
Collaborator

LucieLamothe commented Mar 25, 2022

The 3 added concept seem to be missing parent concept.
Also in EDAM the link "is_format_of" need to refer to an EDAM concept.

@cb2993
Copy link
Author

cb2993 commented Mar 30, 2022

I have added the concepts to the 3 formats by commentary.

For now, I have used the concept "Chemical data format" to hkl, as there are no x-ray data formats available yet. (I hope this is fine, I couldn't find a better solution.)

@sneumann
Copy link

Hi @cb2993 , you can simply edit your checked out version, and push that to your branch. This will automagically update the PR to include those changes. Yours, Steffen

@sonarcloud
Copy link

sonarcloud bot commented Mar 30, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@cb2993
Copy link
Author

cb2993 commented Mar 30, 2022

Thanks for the tip @sneumann , I have changed it accordingly.

Copy link
Member

@hmenager hmenager left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution @cb2993! Can you remove the additional superclass from format_4033 as requested in the comment? I do not see any other issue.

<oboInOwl:inSubset rdf:resource="http://purl.obolibrary.org/obo/edam#edam"/>
<oboInOwl:inSubset rdf:resource="http://purl.obolibrary.org/obo/edam#formats"/>
<oboInOwl:hasDefinition>A flexible tab-delimited reporting data standard file format for sharing quantitative mass spectrometry metabolomics/lipodomics results. It contains the ability to represent “intermediate” or supporting data, including the evidence trail for identifications from software (scores or statistics), as well as the quantification values derived directly from MS: i.e., prior to any adduct grouping or summarization steps. It is an expansion/improvement of mzTab.</oboInOwl:hasDefinition>
<rdfs:subClassOf rdf:resource=”http://edamontology.org/format_3245”/>
Copy link
Member

Choose a reason for hiding this comment

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

There is no need for this format to be subclass of http://edamontology.org/format_3245, because http://edamontology.org/format_3681 is already a subclass of it.

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.

None yet

4 participants