bids-standard / BEP028_BIDSprov

Organizing and coordinating BIDS extension proposal 28 : BIDS Provenance
https://bids.neuroimaging.io/bep028
Creative Commons Attribution 4.0 International
4 stars 12 forks source link

FSL parser v0 #51

Closed remiadon closed 1 year ago

remiadon commented 3 years ago

This PR intoduces a parser script, in bids_prov/fsl_parser.py, so one can automatically transpile .md files as those generated by FSL, into .json sidecar files, as those required by BIDS-prov

TODO

remiadon commented 3 years ago

a v0 is already there

many comments:

  1. What level to choose for an Activity ? Is it a single bash command ? an entire Markdown chapter ?
  2. For now markfown files are handled, but very different file types are presents in NIDMresults-examples
remiadon commented 3 years ago

Here is the first graph I got (quite big I admit) tests

remiadon commented 3 years ago

@cmaumet considering the graph I attached above, which is impressively big, I suggest we opt for attaching a single activity to a single section in the logs file

remiadon commented 3 years ago

@cmaumet as discussed here is an - buggy- example when I try to use the prov library

>> from prov.model import ProvDocument
>> d = ProvDocument()
>> d.add_namespace("prov", "http://www.w3.org/ns/prov#")
>> d.agent(identifier="RRID:SCR_002823")
---------------------------------------------------------------------------
ProvElementIdentifierRequired             Traceback (most recent call last)
<ipython-input-16-32e33baeea1d> in <module>
----> 1 d.agent(identifier="SCR_002823")

/usr/local/Caskroom/miniconda/base/envs/bids-prov/lib/python3.6/site-packages/prov/model.py in agent(self, identifier, other_attributes)
   1835             of tuples to be added to the record optionally (default: None).
   1836         """
-> 1837         return self.new_record(PROV_AGENT, identifier, None, other_attributes)
   1838 
   1839     def attribution(self, entity, agent, identifier=None, other_attributes=None):

/usr/local/Caskroom/miniconda/base/envs/bids-prov/lib/python3.6/site-packages/prov/model.py in new_record(self, record_type, identifier, attributes, other_attributes)
   1603             )
   1604         new_record = PROV_REC_CLS[record_type](
-> 1605             self, self.valid_qualified_name(identifier), attr_list
   1606         )
   1607         self._add_record(new_record)

/usr/local/Caskroom/miniconda/base/envs/bids-prov/lib/python3.6/site-packages/prov/model.py in __init__(self, bundle, identifier, attributes)
    558         if identifier is None:
    559             # All types of PROV elements require a valid identifier
--> 560             raise ProvElementIdentifierRequired()
    561 
    562         super(ProvElement, self).__init__(bundle, identifier, attributes)

ProvElementIdentifierRequired: An identifier is missing. All PROV elements require a valid identifier.
remiadon commented 3 years ago

@cmaumet I've been doing some modifications

This gives the following graph produced_test

remiadon commented 3 years ago

@remiadon - thanks for this. I think we need a bit more documentation before I can review this further. Could you:

* Update the top comment in this PR to describe what the PR is doing.

* Add some comments to the CI tests, at minima a description of what each test is doing.

This will help me (and future readers of this repo) better understanding what the code is doing.

@cmaumet both have been done :)

remiadon commented 3 years ago

@cmaumet I just fixed merged conflicts with the newest master branch

Can we merge this before we integrate the new ontology ? Thx

cmaumet commented 3 years ago

@remiadon -- as discussed above can you add some comments (e.g. in https://github.com/bids-standard/BEP028_BIDSprov/blob/a902c3def20bcbf81fb57ac5a98bd16fd1f6de09/bids_prov/fsl_parser.py) so that the code is easily to understand later on? And then +1 to merge.

remiadon commented 3 years ago

@cmaumet done in the last commit !

thomasbtnfr commented 1 year ago

I close this PR. The current FSL parser is based on this PR but has since evolved.