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

Changed arg scale to data_scale in run_deepcell_task BatchProcessingJob #328

Merged
merged 6 commits into from
Nov 12, 2020

Conversation

awedwards
Copy link
Contributor

This PR will fix and close #327 . The deepcell ```BatchProcessingJobManager```` is expecting the keyword "data_scale" and not "scale". It is also expecting a float and not a string. This PR changes both of these issues.

How did you implement your changes

I changed the argument name scale to data_scale in the run_deepcell_task method in utils.deepcell_service_utils.py. I tested the changes on two different images by changing and comparing the scale to 0.5, 1 and 2.0. Not sure why I saw differences when I originally added this functionality -- when I re-ran the same code I got different results (in other words, the deepcell output wasn't changing even if I changed the scale).

@ngreenwald
Copy link
Member

What happens if you pass a string instead of a float?

@awedwards
Copy link
Contributor Author

It gives the correct output if you input a string. In other words, if you pass something like scale="2.0" you get the same output as scale=2.0.

Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Okay, in that case, can you add a step within create_deepcell_output that attempts to cast scale to a float if it isn't already one, and raises an informative value error if the input can't be cast to a float? And add a check to the test function that the appropriate value error is raised. You can take a look at some of our other functions where we do input validation and pytest.raises() checks in the test functions as an example

@awedwards
Copy link
Contributor Author

I added a try/except clause to check whether or not the scale arg can be cast in deepcell_service_utils and added a test in the deepcell_service_utils_test script.

@ngreenwald ngreenwald merged commit 41f51bf into master Nov 12, 2020
@ngreenwald ngreenwald deleted the scale_arg_debug branch November 12, 2020 03:39
alex-l-kong pushed a commit that referenced this pull request Jan 14, 2021
…ob (#328)

* changed arg scale to data_scale in run_deepcell_task BatchProcessingJobManager call

* added a check to see that scale arg can be converted to float in deepcell_service_utils, added a corresponding test in deepcell_service_utils_test

* style fixes

* second round of style changes
y2kbugger pushed a commit that referenced this pull request Jul 29, 2021
…ob (#328)

* changed arg scale to data_scale in run_deepcell_task BatchProcessingJobManager call

* added a check to see that scale arg can be converted to float in deepcell_service_utils, added a corresponding test in deepcell_service_utils_test

* style fixes

* second round of style changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

scale arg name needs to be changed to data_scale in the call to BatchProcessingJobManager
2 participants