Skip to content
This repository was archived by the owner on Sep 24, 2025. It is now read-only.

Dbodor/prospector fixes#95

Open
DaniBodor wants to merge 8 commits intodbodor/tdd_collapsedfrom
dbodor/prospector_fixes
Open

Dbodor/prospector fixes#95
DaniBodor wants to merge 8 commits intodbodor/tdd_collapsedfrom
dbodor/prospector_fixes

Conversation

@DaniBodor
Copy link
Copy Markdown

@DaniBodor DaniBodor commented Jul 8, 2023

prospector found a bunch of linting issues, which I (mostly) resolved here.

A few remaining that I wasn't sure of are tagged in comments below

I accidentally branched this off another branch, so will point this PR to there for now.

Comment thread cvasl/carve.py Outdated
self.reader.SetFileName(dicom_file)
dcm = self.reader.Execute()
return sitk.GetArrayFromImage(image)
return sitk.GetArrayFromImage(dcm) #TODO check that dcm is correct parameter instead of `image`
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I assume this is what you meant, correct?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should not do anything on any function that deals with images directly, because we don't have them. You are probably right, but this worked on some version of an image I had. Please hold off until we have our actual images to repair carve.

Comment thread cvasl/cli.py Outdated


def main(argv):
def main(argv): #pylint: disable=unused-argument
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

is it required to pass an argument here?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is how I am used to dealing with command-line argparse. I suppose there could be some clever way to write it without an argument,,, but I don't know it.

Comment thread cvasl/mold.py Outdated
array = n4_debias_sitk(file)
elif algorithm == 'alternative_debias_a':
array = alternative_debias_a(file)
array = alternative_debias_a(file) #TODO: what is this?
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this function doesn't exist. Is it a future to do? In that case, shall we just raise a NotImplementedError for now and add a #TODO about it?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, exactly @DaniBodor it's a function we didn't write. And since it deals with images, we should hold off until we are working on images.

@DaniBodor DaniBodor changed the base branch from main to dbodor/tdd_collapsed July 8, 2023 08:04
@DaniBodor DaniBodor changed the base branch from dbodor/tdd_collapsed to drcandacemakedamoore/tdd July 8, 2023 08:05
@DaniBodor DaniBodor changed the base branch from drcandacemakedamoore/tdd to dbodor/tdd_collapsed July 8, 2023 08:05
@drcandacemakedamoore
Copy link
Copy Markdown
Collaborator

You can also just run python setup.py lint (or if you have just done it python setup.py lint --fast). I will lint the other branch you made now, and the rest of these issues should wait until we are dealing with our specific images, so we implement the right thing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants