Skip to content

Conversation

ptrgags
Copy link
Owner

@ptrgags ptrgags commented Dec 20, 2024

This PR:

  • Adds a new 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

Copy link
Owner Author

@ptrgags ptrgags left a 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:
Copy link
Owner Author

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:
Copy link
Owner Author

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)

Copy link
Owner Author

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

Comment on lines +68 to +76
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:
Copy link
Owner Author

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()

Copy link
Owner Author

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.

Copy link
Owner Author

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)
Copy link
Owner Author

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.

Comment on lines +68 to +76
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:
Copy link
Owner Author

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.

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.

1 participant