standardize naming and packaging#5
Conversation
Summary of ChangesHello @yibeichan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the project's consistency and user-friendliness by standardizing its naming conventions. The changes streamline how the application is referenced across container images, command-line interfaces, the Python package, and internal logging, aiming to reduce confusion and improve overall usability for both developers and end-users. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request does a great job of standardizing naming conventions across the project, which significantly improves consistency. The changes to Docker/Singularity image names, command-line usage, and logging are all well-aligned with this goal. I've included a couple of suggestions to further improve maintainability by using constants for the newly standardized names, which should make any future updates even easier.
setup.py
Outdated
| subprocess.run(["docker", "build", "-t", "ants-nidm_bidsapp:latest", "."], check=True) | ||
| print("Docker image built successfully: ants-nidm_bidsapp:latest") | ||
| except subprocess.CalledProcessError as e: | ||
| print(f"Docker build failed: {e}") | ||
| return False | ||
| return True | ||
|
|
||
|
|
||
| def docker_to_singularity(docker_image="ants-nidm-bidsapp:latest", output_path=None): | ||
| def docker_to_singularity(docker_image="ants-nidm_bidsapp:latest", output_path=None): | ||
| """Convert Docker image to Singularity container""" | ||
| print(f"Converting Docker image {docker_image} to Singularity...") | ||
|
|
||
| # Use custom output path if provided, otherwise use default | ||
| output_file = output_path if output_path else "ants-nidm-bidsapp.sif" | ||
| output_file = output_path if output_path else "ants-nidm_bidsapp.sif" |
There was a problem hiding this comment.
The image name ants-nidm_bidsapp and related file names are hardcoded in several places. To improve maintainability, consider defining these as constants at the top of the file. This will make future changes easier and less error-prone.
For example:
DOCKER_IMAGE_NAME = "ants-nidm_bidsapp"
DOCKER_IMAGE_TAG = "latest"
DOCKER_IMAGE = f"{DOCKER_IMAGE_NAME}:{DOCKER_IMAGE_TAG}"
SINGULARITY_IMAGE_NAME = f"{DOCKER_IMAGE_NAME}.sif"You can then use these constants in build_docker, docker_to_singularity, build_singularity, and print_usage functions.
There was a problem hiding this comment.
Pull request overview
This pull request standardizes naming conventions across the project by consolidating various naming patterns (ants-nidm-bidsapp, ants-bidsapp) into a consistent scheme: ants-nidm for the command-line tool and Python package, and ants-nidm_bidsapp for container images.
Key Changes:
- Simplified command-line entry point from
ants-nidm-bidsapptoants-nidmfor improved user experience - Standardized container image names to
ants-nidm_bidsappacross Docker and Singularity - Updated Python package name and logger names throughout the codebase to match the new convention
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/run.py | Updated logger name to ants-nidm and log file naming to use ants-nidm-{timestamp}.log |
| src/antspy/wrapper.py | Updated logger name from ants_bidsapp.segmentation to ants-nidm.segmentation |
| setup.py | Changed package name to ants-nidm, entry point to ants-nidm, and all container image references to ants-nidm_bidsapp |
| examples/README.md | Updated example command from ants-bidsapp to ants-nidm |
| Singularity | Updated container image name in example usage to ants-nidm_bidsapp.sif |
| README.md | Standardized all Docker/Singularity commands to use ants-nidm_bidsapp and CLI examples to use ants-nidm |
| Dockerfile | Updated entrypoint script to call ants-nidm instead of ants-nidm-bidsapp |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This pull request standardizes the naming conventions across the project by replacing all instances of
ants-nidm-bidsappand similar variants withants-nidmorants-nidm_bidsapp, depending on the context. This affects Docker/Singularity image names, command-line usage, logging, and the Python package name, resulting in more consistent and user-friendly commands and documentation.Container and Command Naming Updates:
Changed all Docker and Singularity image names and related commands in
README.md,setup.py, andSingularityfromants-nidm-bidsapptoants-nidm_bidsappfor clarity and consistency. This includes build, save, and run commands, as well as references in documentation and example usage. [1] [2] [3] [4] [5] [6] [7] [8]Updated the command-line entry point and all usage examples from
ants-nidm-bidsapporants-bidsapptoants-nidmfor a simpler and more intuitive user experience. [1] [2] [3] [4]Python Package and Logging Renaming:
setup.pyfromants-nidm_bidsapptoants-nidmand updated the logger names throughout the codebase to match the new naming convention. [1] [2] [3] [4]Entrypoint and Script Updates:
ants-nidminstead of the oldants-nidm-bidsappexecutable.These changes ensure that the project uses a consistent and simplified naming scheme, which will reduce confusion for users and developers alike.