sabeenlohawala / tissue_labeling

0 stars 1 forks source link

simple code suggestions to improve #50

Open hvgazula opened 8 months ago

hvgazula commented 8 months ago

https://github.com/sabeenlohawala/tissue_labeling/blob/8611ada4596771e1ab25cb02faf7be557509593b/scripts/mit_kwyk_data.py#L150

dict(zip(unique,count))

https://github.com/sabeenlohawala/tissue_labeling/blob/8611ada4596771e1ab25cb02faf7be557509593b/scripts/mit_kwyk_data.py#L180C1-L181C85

shapes, pixel_counts = zip(*shapes_and_pixel_counts)

https://github.com/sabeenlohawala/tissue_labeling/blob/8611ada4596771e1ab25cb02faf7be557509593b/scripts/mit_kwyk_data.py#L249-L292

Run this example and tell me if the above cannot be improved in the same way

import numpy as np
a = np.random.rand(10, 5, 3)
b  = list(map(sum, a)  # sum can be any function
print(len(b), b[0].shape)
hvgazula commented 8 months ago

https://github.com/sabeenlohawala/tissue_labeling/blob/8611ada4596771e1ab25cb02faf7be557509593b/scripts/mit_kwyk_data.py#L229

uint16 will do for the label vols

edit: please see the table at the bottom of this page

hvgazula commented 8 months ago

https://github.com/sabeenlohawala/tissue_labeling/blob/8611ada4596771e1ab25cb02faf7be557509593b/scripts/mit_kwyk_data.py#L183

couldn't this be written as {*d.keys() for d in pixel_counts}?

update: iterable unpacking cannot be used in comprehension

hvgazula commented 8 months ago

https://github.com/sabeenlohawala/tissue_labeling/blob/8611ada4596771e1ab25cb02faf7be557509593b/scripts/mit_kwyk_data.py#L392-L393

Not sure if I agree with this. What if the middle value is the largest? You will end up with a square and that's unnecessary. Am I missing something?

hvgazula commented 8 months ago

https://github.com/sabeenlohawala/tissue_labeling/blob/8611ada4596771e1ab25cb02faf7be557509593b/scripts/mit_kwyk_data.py#L379-L381

does this have to be done within the context manager?

hvgazula commented 8 months ago

Also, pixel_counts is a dict. Could you not simply write sum(pixel_counts), although i am not sure yet why the keys are added and not the values?

sabeenlohawala commented 8 months ago

https://github.com/sabeenlohawala/tissue_labeling/blob/8f9b20506740c2364051e9ca6975efd7f7ace38b/scripts/mit_kwyk_data.py#L293C9-L298C436

https://github.com/sabeenlohawala/tissue_labeling/blob/8f9b20506740c2364051e9ca6975efd7f7ace38b/scripts/mit_kwyk_data.py#L411

Using list(map(...)) slows down the computation, but removing list results in error thrown in line 411: 'map' object is not subscriptable.

sabeenlohawala commented 8 months ago

https://github.com/sabeenlohawala/tissue_labeling/blob/8611ada4596771e1ab25cb02faf7be557509593b/scripts/mit_kwyk_data.py#L392-L393

Not sure if I agree with this. What if the middle value is the largest? You will end up with a square and that's unnecessary. Am I missing something?

slice[i,:,:] → shape is dim[1] x dim[2] slice[:,i,:] → shape is dim[0] x dim[2] slice[:,:,i] → shape is dim[0] x dim[1] Therefore, in order for all slices to be the same shape, slice shape should be (max(dim[0], dim[1]), max(dim[1], dim[2]))

hvgazula commented 8 months ago

cool..please create a separate function with this docstring so people like me will know why 😄