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

Draft CircleCI configuration #43

Merged
merged 15 commits into from
Aug 28, 2024
Merged

Draft CircleCI configuration #43

merged 15 commits into from
Aug 28, 2024

Conversation

tsalo
Copy link
Collaborator

@tsalo tsalo commented Aug 7, 2024

Closes #42.

Pending issues:

  • ruff doesn't like how I have local testing set up.
  • I need a nipreps admin to enable this project on CircleCI.
  • I need help getting the Docker deployment step working.

Changes proposed in this pull request

  • Try creating a CircleCI config file.
  • Update documentation to reflect that output spaces aren't supported yet.
  • Remove config.execution.fmripost_aroma_dir and just use config.execution.output_dir.
  • Add CLI smoke tests.

Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 15.50388% with 109 lines in your changes missing coverage. Please review.

Project coverage is 30.54%. Comparing base (4bd4e0c) to head (550be7f).
Report is 29 commits behind head on main.

Files with missing lines Patch % Lines
src/fmripost_aroma/tests/utils.py 18.51% 88 Missing ⚠️
src/fmripost_aroma/workflows/base.py 0.00% 12 Missing ⚠️
src/fmripost_aroma/cli/run.py 0.00% 3 Missing ⚠️
src/fmripost_aroma/cli/workflow.py 0.00% 2 Missing ⚠️
src/fmripost_aroma/utils/bids.py 0.00% 1 Missing and 1 partial ⚠️
src/fmripost_aroma/cli/parser.py 0.00% 1 Missing ⚠️
src/fmripost_aroma/workflows/resampling.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
- Coverage   31.18%   30.54%   -0.64%     
==========================================
  Files          26       26              
  Lines        2155     2259     +104     
  Branches      327      358      +31     
==========================================
+ Hits          672      690      +18     
- Misses       1463     1549      +86     
  Partials       20       20              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tsalo
Copy link
Collaborator Author

tsalo commented Aug 24, 2024

@effigies would you be willing to activate fmripost-aroma on CircleCI?

@effigies
Copy link
Member

Unfortunately, we need to hear back about a no-cost extension before committing more money to CircleCI. Would you be willing to run it on your fork?

@tsalo
Copy link
Collaborator Author

tsalo commented Aug 28, 2024

I can do that, although I'm not sure how the Docker deployment would work.

@tsalo
Copy link
Collaborator Author

tsalo commented Aug 28, 2024

I can just have my fork deploy I guess. I am worried about deploying from tags, but I can just push to unstable for now.

@tsalo tsalo merged commit 60fbfb2 into nipreps:main Aug 28, 2024
9 of 12 checks passed
@tsalo tsalo deleted the draft-circleci branch August 28, 2024 16:19
@effigies
Copy link
Member

For docker, we could deploy via GitHub actions.

@tsalo
Copy link
Collaborator Author

tsalo commented Aug 28, 2024

That works for me. Can you either add me to the nipreps org on DockerHub (username tsalo) or add your credentials as secrets on this repo? The action I'm adding in #46 needs DOCKER_USERNAME and DOCKER_PASSWORD.

@@ -207,6 +207,15 @@ def init_single_subject_wf(subject_id: str):
# Patch standard-space BOLD files into 'bold' key
subject_data['bold'] = listify(subject_data['bold_mni152nlin6asym'])

if not subject_data['bold_mni152nlin6asym']:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like this was a mistake for raw+derivatives processing, because the derivatives datasets weren't being queried.

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.

Testing strategy
2 participants