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

ENH: Build bundle list dynamically depending on atlas version #133

Merged

Conversation

jhlegarreta
Copy link
Contributor

@jhlegarreta jhlegarreta commented Jul 15, 2023

Build bundle list dynamically depending on atlas version.

Add the accompanying files:

  • The atlas bundle data (bundle label, short and long names) as CSV files.
  • The utilities functions required to read the bundle data.
  • Provide some level of encapsulation when adding the methods.

Add a utility script to query the bundles corresponding to the files in an atlas available in a local disk.

Add the corresponding tests.

@jhlegarreta
Copy link
Contributor Author

jhlegarreta commented Jul 15, 2023

A few notes:

  • Once PR ENH: Allow specifying the atlas version at download #132 gets hopefully merged, some code could be refactored so that we have a single enum class that will contain all necessary information, including the zenodo records, and the local CSV files added in this PR.
  • Part of this code would maybe fits better in https://github.com/SlicerDMRI/ORG-Atlases, but adding this to there would require building a package from it, which is not desirable given the current state of things. Also, because that repository only contains documentation and bundle files.
  • I am adding the atlas bundle lists here. It would probably better to have them added to the zenodo versions, but probably zenodo will create new versions, which we do not want I think.

While doing this I've realized that we should explain the below:

  • Atlas version 1.1 (record: 2648284)
    • ORG-800FC-100HCP.zip: only contains the 800 cluster files
    • ORG-800FiberClusters.zip: does not contain bundle TT
  • Atlas version 1.1.1 (record: 2648292)
    • ORG-800FC-100HCP.zip: only contains the 800 cluster files
    • ORG-800FiberClusters.zip: does not contain bundle TT
  • Atlas version 1.2 (record: 4156927)
    • ORG-800FC-100HCP.zip: only contains the 800 cluster files
    • ORG-800FiberClusters.zip: does not contain bundle TT
  • Atlas version 1.3a (record: 5109770)
    • ORG-800FC-100HCP.zip: contains bundle TT
    • ORG-800FC-100HCP-separated/AnatomicalTracts: does not contain bundle TT
  • Atlas version 1.3b (record: 778496)
    • ORG-800FC-100HCP.zip: contains bundle TT
    • ORG-800FC-100HCP-separated.zip/AnatomicalTracts: does not contain bundle TT
  • Atlas version 1.4 (record: 8082481)
    • ORG-800FC-100HCP.zip: contains bundle TT
    • ORG-800FC-100HCP-separated.zip/AnatomicalTracts: does not contain bundle TT

I have not looked into the 2000FC folders, or the MRB, MRML files, but according to the instructions we would download the ORG-800FC-100HCP.zip file, but as said in #126 (comment), the bundle list was being hard-coded, and not including TT. It is confusing at what the code is looking at/analyzing also because the structure and even the contents of the equivalent ZIP files have changed across versions. I have not looked either into whether the data in the ORG-Atlases repository is consistent with the zenodo records. Can all this be explained/clarified in the documentation, please? Also, if some of these records are incomplete or inconsistent, IMO they should be removed.

@jhlegarreta
Copy link
Contributor Author

An additional notes about the feature itself:

Thus, it is better to wait for PR #134 to be merged before merging this so that we ensure that the modules can be imported. Will rebase on master once the former is merged.

@jhlegarreta jhlegarreta marked this pull request as draft July 16, 2023 16:34
@jhlegarreta jhlegarreta force-pushed the BuildBundleListDynamically branch 5 times, most recently from c5f581e to ffcd6a1 Compare October 12, 2023 16:22
@jhlegarreta jhlegarreta force-pushed the BuildBundleListDynamically branch 7 times, most recently from db00c98 to a88a759 Compare October 20, 2023 16:48
@jhlegarreta jhlegarreta marked this pull request as ready for review October 20, 2023 16:50
@jhlegarreta
Copy link
Contributor Author

@zhangfanmark please comment on #133 (comment).

Build bundle list dynamically depending on atlas version.

Add the accompanying files:
- The atlas bundle data (bundle label, short and long names) as CSV
  files.
- The utilities functions required to read the bundle data.
- Provide some level of encapsulation when adding the methods.

Add a utility script to query the bundles corresponding to the files in
an atlas available in a local disk.

Add the corresponding tests.
@jhlegarreta jhlegarreta merged commit 8142c36 into SlicerDMRI:master Oct 21, 2023
9 checks passed
@jhlegarreta jhlegarreta deleted the BuildBundleListDynamically branch October 21, 2023 19:39
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.

None yet

1 participant