-
Notifications
You must be signed in to change notification settings - Fork 1.2k
followup of #1878 #1913
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
followup of #1878 #1913
Conversation
Signed-off-by: Wenqi Li <[email protected]>
/black |
Hi @wyli, why are you removing the json loading part? For pathology, since this metric relies on all the ground truth masks and all the probability masks, it should be called independently after all the inferences are done, and they need be defined in a json file. |
yes before the data working group provides a concrete solution we shouldn't couple the lesion evaluation module with json encoding/decoding logic. if it's really needed please write a separate json loader. This class should only work with |
Signed-off-by: Behrooz <[email protected]>
thanks for the update @behxyz! there's an example of deepgrow with a demo script to show a task-specific json loader https://github.com/Project-MONAI/tutorials/blob/55006d748626f6af59567a295725ccbd3bdd6bef/deepgrow/ignite/train.py#L315 |
OK, I'll add a separate json reader till we have an agreed upon solution. Would this be the topic of data WG or pathology WG? |
I updated the test case number to be in an order and committed to your PR. |
* followup of Project-MONAI#1878, fixes tests, remove json loading Signed-off-by: Wenqi Li <[email protected]> * Update test ordinal numbers Signed-off-by: Behrooz <[email protected]> Co-authored-by: Behrooz <[email protected]> Signed-off-by: Neha Srivathsa <[email protected]>
update tests (
TEST_CASE_5
duplicated), remove json loading (#1878 (comment))Status
Ready
Types of changes