Skip to content

Add more integration tests for run_nifti_insertion.py #1266

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

Merged
merged 25 commits into from
Apr 14, 2025

Conversation

cmadjar
Copy link
Collaborator

@cmadjar cmadjar commented Apr 11, 2025

No description provided.

@cmadjar cmadjar added Blocked Merge it and you die Area: CI PR or issue related to continuous integration, including automated tests and static checks labels Apr 11, 2025
@cmadjar cmadjar added this to the 27.0 milestone Apr 11, 2025
@cmadjar cmadjar self-assigned this Apr 11, 2025
@cmadjar cmadjar closed this Apr 11, 2025
@cmadjar cmadjar reopened this Apr 11, 2025
@@ -175,7 +176,8 @@ def __init__(self, loris_getopt_obj, script_name):
# ---------------------------------------------------------------------------------------------
# Create the pic images
# ---------------------------------------------------------------------------------------------
self._create_pic_image()
if self.create_pic_bool:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

bug found when implementing the tests 🥳

@cmadjar cmadjar removed Needs Work Before Merging Blocked Merge it and you die labels Apr 12, 2025
@cmadjar cmadjar requested a review from maximemulder April 12, 2025 13:26
Copy link
Contributor

@maximemulder maximemulder left a comment

Choose a reason for hiding this comment

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

Wow, those are certainly very exhaustive tests ! Congrats for the work !

Two minor comments:

  • Query function naming.
  • Some unnecessary None-checks that can be removed.

If you agree with those, just apply the advised changes and you can merge without re-review.

@cmadjar cmadjar merged commit 1ff36ce into aces:main Apr 14, 2025
9 checks passed
@cmadjar cmadjar added the Area: ORM PR or issue related to the SQLAlchemy integration label Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CI PR or issue related to continuous integration, including automated tests and static checks Area: ORM PR or issue related to the SQLAlchemy integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants