Skip to content

Conversation

@SamChou05
Copy link
Collaborator

No description provided.

@arokem
Copy link
Member

arokem commented Jan 20, 2025

@SamChou05 : now that #19 is merged, please go ahead and merge the main branch into your branch or rebase on top of main so that we can see whether your tests will pass.

@SamChou05 SamChou05 self-assigned this Mar 31, 2025
@arokem
Copy link
Member

arokem commented Mar 31, 2025

The code in pt_models looks great!

I wonder whether prep_first_tract_data, prep_fa_dataset, and prep_fa_flattned_data are all a bit specific to the analysis that you are currently doing, so maybe they belong in your analysis code repo instead of here?

@SamChou05
Copy link
Collaborator Author

I agree with your comment with prep_first_tract_data, prep_fa_dataset, and prep_fa_flattned_data, so I moved them to my repository containing the experiments and removed them from this PR.

@arokem
Copy link
Member

arokem commented Apr 2, 2025

Great! You'll need to move out the related test as well

@arokem
Copy link
Member

arokem commented Apr 2, 2025

Could you please add docstrings to all the new functions/classes?

@arokem arokem merged commit 730b16e into tractometry:main Apr 2, 2025
3 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.

2 participants