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

SPM parser config is now external #54

Closed remiadon closed 3 years ago

remiadon commented 3 years ago

config in separate file : bids_prov/spm_config.py

This includes

In other words, everything but the algorithmic logic in the parser

remiadon commented 3 years ago

@cmaumet there are no configuration files stored. This is the SPM parser, everything lies in the spm_config.py file

remiadon commented 3 years ago

@cmaumet j'ai fait un premier exemple avec l'activité "segment"

Voici le nouveau graphe test

Plusieurs questions:

J'espère que c'est bien ce format là que tu attends

remiadon commented 3 years ago

test @cmaumet voici le graphe que j'obtiens en lançant python -m bids_prov.spm_parser examples/spm_default/batch.m -o test.json && python bids_prov/visualize.py --omit-details test.json && open test.png depuis la racine

cmaumet commented 3 years ago

Ok - as-tu vérifié si la dépendance qui se trouve ici est bien retrouvée par ton code ici ? Par exemple en ajoutant un print(dependency) pour voir quelles sone les dépendences qui sont bien trouvées ?

remiadon commented 3 years ago

@cmaumet oui il y est bien

cmaumet commented 3 years ago

Très bien ! Et est-ce que le lien "used" est bien créé ici? Si oui, avec quelle activité ?

remiadon commented 3 years ago

@cmaumet le lien "used" est mauvais, cf ici https://github.com/bids-standard/BEP028_BIDSprov/blob/56c71878ab550637d2fe340112f1551cfd07d946/bids_prov/spm_parser.py#L150

la variable "output_id" n'est peut etre pas le bon id ....

cmaumet commented 3 years ago

Remi: I think I've found a solution. The link created by the line you mentioned above was correct but then it was overwritten at: https://github.com/bids-standard/BEP028_BIDSprov/blob/56c71878ab550637d2fe340112f1551cfd07d946/bids_prov/spm_parser.py#L171-L172

I've proposed a fix to instead merge the two lists (cf. https://github.com/bids-standard/BEP028_BIDSprov/pull/54/commits/0b24cabb85c8f8cd3ccbb2c5af3222e6e25fb60f) and now the link between "smoothed images" and "fmri_spec" is no longer missing.

test