Conversation
ptrgags
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
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.
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.
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.
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.
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:
statscommand 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)Masknow uses these stats in the constructor