Skip to content

Conversation

mathdugre
Copy link
Collaborator

@mathdugre mathdugre commented May 6, 2025

Changes proposed in this pull request:

Checklist (for reviewers)

This section is for the PR reviewer

  • PR has an interpretable title with a prefix (e.g. [BUG], [DOC], [ENH], [MAINT])
    Refer to NumPy Development Guide for a full list
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions

📚 Documentation preview 📚: https://nipoppy--626.org.readthedocs.build/en/626/

@mathdugre mathdugre marked this pull request as ready for review May 6, 2025 16:25
@mathdugre mathdugre requested a review from michellewang May 6, 2025 16:25
@nipoppy-bot nipoppy-bot bot added this to Software May 6, 2025
@nipoppy-bot nipoppy-bot bot moved this to Pending review in Software May 6, 2025
Copy link

codecov bot commented May 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
nipoppy/cli.py 99.49% <100.00%> (-0.51%) ⬇️
nipoppy/workflows/pipeline_store/zenodo.py 93.33% <100.00%> (-6.67%) ⬇️

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sandbox=params.pop("sandbox"),
access_token=params.pop("access_token"),
)
with handle_exception(PipelineSearchWorkflow(**params)) as workflow:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we have the Zenodo-related workflows accept sandbox and password_file directly (instead of the Zenodo API)? So that the CLI command does exactly the same thing as the workflow class

@github-project-automation github-project-automation bot moved this from Pending review to Reviewed in Software May 6, 2025
@mathdugre mathdugre requested a review from michellewang May 6, 2025 20:57
@nipoppy-bot nipoppy-bot bot moved this from Reviewed to Pending review in Software May 6, 2025
Copy link
Collaborator

@michellewang michellewang left a comment

Choose a reason for hiding this comment

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

There is a failing test due to nested quotes but I think I can fix it

@github-project-automation github-project-automation bot moved this from Pending review to Approved in Software May 6, 2025
Comment on lines +88 to +90
if not continue_:
self.logger.info("Zenodo upload cancelled.")
raise SystemExit(1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

🍒 we could mock this to get 100% test coverage

@mathdugre mathdugre requested a review from michellewang May 8, 2025 17:23
@nipoppy-bot nipoppy-bot bot moved this from Approved to Pending review in Software May 8, 2025
Copy link
Collaborator

@michellewang michellewang left a comment

Choose a reason for hiding this comment

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

🧑‍🍳

@github-project-automation github-project-automation bot moved this from Pending review to Approved in Software May 8, 2025
@michellewang michellewang merged commit e2f73df into nipoppy:main May 8, 2025
11 checks passed
@nipoppy-bot nipoppy-bot bot moved this from Approved to Done in Software May 8, 2025
@mathdugre mathdugre deleted the enh/zenodo branch May 8, 2025 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants