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

Refactor remaining array configs to use ConfigOB instead of the DBI library #770

Merged
merged 11 commits into from
Feb 17, 2023

Conversation

cmadjar
Copy link
Collaborator

@cmadjar cmadjar commented Mar 16, 2022

This PR finishes the refactoring of the database config queries to use ConfigOB instead of the DBI library directly.

This resolves #465

Copy link
Collaborator

@nicolasbrossard nicolasbrossard left a comment

Choose a reason for hiding this comment

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

Haven't tested this yet, just reviewed it. Full Moose compliance for the Config. Moo Moo !!!!

tools/delete_imaging_upload.pl Outdated Show resolved Hide resolved
docs/scripts_md/MRIProcessingUtility.md Outdated Show resolved Hide resolved
@cmadjar cmadjar force-pushed the refactor_remaining_array_configs branch from 40b5542 to b48bcde Compare June 3, 2022 13:29
@cmadjar
Copy link
Collaborator Author

cmadjar commented Jun 3, 2022

@nicolasbrossard ready for another review. Thank you!

@zaliqarosli zaliqarosli self-assigned this Jun 17, 2022
tools/run_defacing_script.pl Outdated Show resolved Hide resolved
uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm Outdated Show resolved Hide resolved
@cmadjar
Copy link
Collaborator Author

cmadjar commented Jun 17, 2022

@nicolasbrossard I believe I took care of all your comments. Thanks!

tools/run_defacing_script.pl Outdated Show resolved Hide resolved
tools/run_defacing_script.pl Outdated Show resolved Hide resolved
docs/scripts_md/run_defacing_script.md Outdated Show resolved Hide resolved
@cmadjar
Copy link
Collaborator Author

cmadjar commented Jun 17, 2022

@nicolasbrossard good catch! Should be fixed in the last commit


Get the bids_acknowledgments_text Config setting

RETURN: an array (possibly empty) of the acknowledgment text to use for a BIDS dataset
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this method returns an array but a string. Also, when you do not define an acknowledgment text, this method returns undef (not the empty string ''). Is this the intended behavior?


Get the bids_readme_text Config setting

RETURN: an array (possibly empty) of the README text to use for a BIDS dataset
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as for the acknowledgment text.

@github-actions
Copy link

github-actions bot commented Dec 1, 2022

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 20 days.

@github-actions github-actions bot added the Stale label Dec 1, 2022
@github-actions github-actions bot closed this Jan 16, 2023
@cmadjar cmadjar reopened this Jan 16, 2023
@cmadjar cmadjar removed the Stale label Jan 16, 2023
@cmadjar cmadjar merged commit 4257108 into aces:main Feb 17, 2023
zaliqarosli pushed a commit to zaliqarosli/Loris-MRI that referenced this pull request Mar 9, 2023
…ibrary (aces#770)

* use configOB for array configs

* use configOB for minc_to_bids script

* fix bug in delete script

* fix bugs with new perl

* use array for some bids config

* bug fix

* fix pod error + Nic's comment

* take care of Nic's comments

* recreate markdown files

* correct documentation of grep_FileIDs_to_deface in run_defacing_script

* Nic's review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grep the configuration by the ConfigOB
3 participants