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

Output None for inexistent parameter #119

Merged
merged 3 commits into from
Feb 5, 2024

Conversation

jeremie-fouquet
Copy link
Contributor

Fixes #100.

Changes proposed in this pull request:

  • Get meta_check_source to output None in case of an inexistent parameter
  • Equivalent to reverting to the previous behaviour in the case of a B0 map, which was to output the string "Value was not specified" for EchoTime in the json file

@BrkRaw/Bruker

Output explanatory string in BIDS json
@dvm-shlee
Copy link
Member

The object of returning the non-existing parameter is to return original input as the value, so if the input value is not parameter related, it will acting as string. I did not deeply review in this changes, but have you tested whether changing this makes string input became None in output json file?

@jeremie-fouquet
Copy link
Contributor Author

Because of this line of code, None is transformed into the string 'Value was not specified' before saving the json. Does that answer the question?

@jeremie-fouquet
Copy link
Contributor Author

See also here for more context.

@dvm-shlee
Copy link
Member

I see your point, but we need to test if the proposed change impacts the hardcoded string in the METADATA_REF.

I recall there being a specific reason for this function to return a string. For instance, the template is designed to use string values that aren't present in the metadata files by default. For example, in /brkraw/lib/reference.py, the FMRI_META_REF has a key "CogAtlasID" where the value is a common string (URL) rather than a parameter. I'm concerned that the change you suggest might result in this value being labeled as "Value was not specified".

@jeremie-fouquet
Copy link
Contributor Author

Ok, I get what you mean. In the commit above, I suggest a quick fix. It will only assign None to a parameter value when it is still a string within the function meta_check_express. I think it addresses your concern; let me know if not.

At a later time, it might be worth it to reconsider how the logic behind these checks is implemented.

@dvm-shlee
Copy link
Member

Based on your suggestion, I'll focus on enhancing the logic BIDS converter for better readability and clarity over the next two weeks. I'll go ahead and merge this pull request. Thanks!

@dvm-shlee dvm-shlee merged commit 6483ce1 into BrkRaw:main Feb 5, 2024
26 checks passed
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.

Current GitHub version fails to process using JSON
2 participants