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

Moved the test files to OSF #88

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Moved the test files to OSF #88

wants to merge 3 commits into from

Conversation

demianw
Copy link
Owner

@demianw demianw commented Jan 16, 2025

Move the test files to OSF. @jhlegarreta can you check if this works with PR #78 . Need to solve the MD5 checking

@jhlegarreta
Copy link
Contributor

Thanks for doing this. Have been investigating: the approach would work with an additional bug fix/change in the tract_math.decorator module. Have tested with a VTK tractogram that I had used locally to test other things in the tool.

So once the hashes are correctly computed (I have seen that all three files uploaded to OSF are, in reality, the same file), and this continue statement removed:

transferring/adapting the test_scripts.py tests to the test files in PR #77 should not be difficult and should allow us to have these tests running/passing in the CI so that we can move forward.

@jhlegarreta
Copy link
Contributor

Can the test data be updated in OSF, please? I can amend this PR once the data is hosted in OSF. Thanks.

@jhlegarreta
Copy link
Contributor

Investigated a little more on this.

(I have seen that all three files uploaded to OSF are, in reality, the same file)

The files being downloaded were some metadata files, the same for the different test files, and were thus, not the actual test files. I have cherry-picked the first commit in this PR in #78, have modified the URLs and made a couple more changes, and got the tests passing locally.

Thus, this PR is superseded by PR #78 @demianw.

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