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

Get RICA to work with tedana_reclassify #57

Closed
handwerkerd opened this issue Dec 22, 2022 · 3 comments · Fixed by #58
Closed

Get RICA to work with tedana_reclassify #57

handwerkerd opened this issue Dec 22, 2022 · 3 comments · Fixed by #58

Comments

@handwerkerd
Copy link
Member

Summary

When Decision Tree Modularization is merged, the interface with RICA will break. The current function RICA uses to manually change classifications will be removed and these changes will happen through a new workflow tedana_reclassify It would be good to make sure RICA works with both version before that major change is merged

Additional Detail

RICA should be able to interface with tedana through tedana.workflows.tedana_reclassify.post_tedana. In the new version, instead of inputting multiple filenames, it takes a filename registry called desc-tedana_registry.json or registry.json RICA should be able to use the same system where users select a directory to use, but, if the registry exists, use the registry to identify file names and the run post_tedana otherwise just run the current I/O code which should keep for older versions of tedana.

With the newer version, the component table will keep existing classification_tags for each component and also add a manual reclassify tag. Additionally the component_status_table will add columns for each component showing how components where changed with reclassification so that provenance of manually will be fully documented.

Additionally, the file registry should work with relative paths so it would be possible to have RICA output a new denoised dataset to a new output directory and the new registry in that output directory will be able to reference back to the original files that weren't changed and didn't need to be copied.

Next Steps

  • Everything in the modularization PR should be rest to work wtih RICA.
  • From our last dev call, it sounded like @eurunuela wanted to try to find time to work on this.
  • @jbteves and @handwerkerd can definitely help if there's any confusion or if issues arise.
  • Ideally get this done soonish (January), so, if we need to change anything in tedana to make this work, we can make sure it's fixed before merging that PR.
@eurunuela
Copy link
Collaborator

I've had a look at tedana_reclassify.

It looks like passing the manacc and manrej arguments as a list of integers is necessary, right? If that is the case, I believe the easiest workaround to make it compatible with Rica would be to accept .csv files too. This way, I could generate two .csv files with Rica: one for accepted components, and one for rejected components.

What do you think @handwerkerd @jbteves?

@jbteves
Copy link

jbteves commented Jan 10, 2023

Eneko, I implemented a solution for you and then totally forgot to tell you, sorry. There are a few ways to supply the components now:

  1. supply it as a comma-delimited string on the command line
  2. Supply it as a .csv file, with either no column headers or at least one column with a "Components" header.
  3. Supply it as a text file, with each component number delimited by space, tab, newline, or commas.

@eurunuela
Copy link
Collaborator

This is great. Thank you Josh! I will try to develop the new downloading function in Rica sometime this week.

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 a pull request may close this issue.

3 participants