-
Notifications
You must be signed in to change notification settings - Fork 0
Image Mask Stats #7
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some thoughts on the code so far. It works, but not quite happy with the implementation yet.
return red << 2 | green << 1 | blue | ||
|
||
|
||
def make_index_mask(img_mask: numpy.ndarray) -> numpy.ndarray: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a constructor, make a dedicated IndexMask
type that enforces values are in [0, 7)
|
||
|
||
@dataclass | ||
class IndexMaskStats: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class feels like it mixes the concepts of an image mask (the original image) and the index mask (the array of values in [0, 8)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe there should be two different types of stats objects? And the pixelops stats
command will compute both? Meanwhile Mask
would only use the IndexMaskStats
def compute_index_mask_stats(index_mask: numpy.ndarray) -> IndexMaskStats: | ||
height, width = index_mask.shape | ||
dimensions = (width, height) | ||
|
||
bincounts = numpy.bincount(index_mask.flatten()) | ||
return IndexMaskStats(dimensions, 1, bincounts) | ||
|
||
|
||
def compute_image_mask_stats(img_mask: numpy.ndarray) -> IndexMaskStats: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having dedicated types for an image mask vs an index mask would be nice. Then it could just be mask.stats()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though... since we have Mask
, (which is really an index mask) maybe IndexRaster
vs ColorRaster
? a light wrapper around Numpy arrays whose constructors check the right number of dimensions/channels, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is typing.NewType
that could be used to alias a numpy array...
But also, is Mask
really just the IndexMask
abstraction I want? It could compute more stats. The only thing is that Mask
enforces contiguous indices. Though that ValueError could be caught by the stats
command.
I need to think about this some more. It might also help to look over all the places where I use numpy.ndarray
and determine what are the major use cases. E.g. address buffers is another use case, and then you have the final rendered image.
|
||
expected = make_expected_image(5) | ||
assert (result == expected).all() | ||
assert numpy.array_equal(result, expected) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes to array equal should be done in a separate smaller PR.
def compute_index_mask_stats(index_mask: numpy.ndarray) -> IndexMaskStats: | ||
height, width = index_mask.shape | ||
dimensions = (width, height) | ||
|
||
bincounts = numpy.bincount(index_mask.flatten()) | ||
return IndexMaskStats(dimensions, 1, bincounts) | ||
|
||
|
||
def compute_image_mask_stats(img_mask: numpy.ndarray) -> IndexMaskStats: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is typing.NewType
that could be used to alias a numpy array...
But also, is Mask
really just the IndexMask
abstraction I want? It could compute more stats. The only thing is that Mask
enforces contiguous indices. Though that ValueError could be caught by the stats
command.
I need to think about this some more. It might also help to look over all the places where I use numpy.ndarray
and determine what are the major use cases. E.g. address buffers is another use case, and then you have the final rendered image.
This PR:
stats
command that takes an image or directory and computes statistics about the image, mainly the index counts and index densities (this may be used for better mask selection algorithms in the near future)Mask
now uses these stats in the constructor