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

Introduce segment_image CLI #1254

Merged
merged 8 commits into from Jan 13, 2023
Merged

Introduce segment_image CLI #1254

merged 8 commits into from Jan 13, 2023

Conversation

jcohenadad
Copy link
Member

@jcohenadad jcohenadad commented Jan 3, 2023

Checklist

GitHub

  • I've given this PR a concise, self-descriptive, and meaningful title
  • I've linked relevant issues in the PR body
  • I've applied the relevant labels to this PR
  • I've assigned a reviewer

PR contents

Description

Introduces a CLI function to segment individual images, regardless of the BIDS structure.

Linked issues

Addresses #1002

@jcohenadad jcohenadad marked this pull request as draft January 3, 2023 03:42
@jcohenadad jcohenadad changed the title First prototype for a 'segment_image' CLI Introduce segment_image CLI Jan 3, 2023
@coveralls
Copy link

coveralls commented Jan 3, 2023

Pull Request Test Coverage Report for Build 3893475163

  • 0 of 37 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.4%) to 74.093%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ivadomed/scripts/segment_image.py 0 37 0.0%
Totals Coverage Status
Change from base Build 3850746212: -0.4%
Covered Lines: 4616
Relevant Lines: 6230

💛 - Coveralls

@jcohenadad jcohenadad marked this pull request as ready for review January 4, 2023 02:00
@jcohenadad jcohenadad added the feature category: new functionality label Jan 4, 2023
@jcohenadad jcohenadad added this to the new release milestone Jan 4, 2023
@mariehbourget mariehbourget modified the milestone: new release Jan 4, 2023
Copy link
Member

@naga-karthik naga-karthik left a comment

Choose a reason for hiding this comment

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

Looks good to me! It is an important feature to have in the CLI. I can take up the rest of the TODOs!

This script applies a trained model on a single image. Output are generated in the current directory.
"""

# TODO: create entry_points in setup.py and update docstrings usage
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the other files in the PR, I think this comment can be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed-- sorry i have no more time to dedicate to this-- pls fix it in a subsequent PR

help="Image(s) to segment. You can specify more than one image (separate with space).",
metavar=imed_utils.Metavar.file)
parser.add_argument("-m", "--model", required=True,
help="Path to folder that contains ONNX and/or PT model and ivadomed JSON config file.",
Copy link
Member

Choose a reason for hiding this comment

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

Because this command can be used independently of the standard ivadomed training pipeline, I suggest making it more explicit. Maybe this could be added:

"Path to folder that contains ONNX and/or PT model and ivadomed JSON config file. Can be found inside the model_name folder under path_output used in the JSON config file."

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry i have no more time to dedicate to this-- pls fix it in a subsequent PR

@jcohenadad
Copy link
Member Author

I can take up the rest of the TODOs!

Thank you @naga-karthik , I accept the offer 😊 Let's merge this and I let you open another PR for the remaining TODOs

@jcohenadad
Copy link
Member Author

@naga-karthik i let you open an issue for the remaining comments/TODOs, approve this PR and merge it pls

@naga-karthik
Copy link
Member

Done! #1270 lists the TODOs as remaining tasks.

@naga-karthik naga-karthik merged commit fbc97b8 into master Jan 13, 2023
@naga-karthik naga-karthik deleted the jca/1002-segment-cli branch January 13, 2023 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature category: new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants