Expression icon indicating copy to clipboard operation
Expression copied to clipboard

Add `NonEmptyBlock` class and utility functions

Open brendanmaguire opened this issue 2 years ago • 4 comments

  • This collection guarentees to have at least one element
  • Uses Scala cats NonEmptyList as inspiration

Closes #177

brendanmaguire avatar Nov 11 '23 15:11 brendanmaguire

~This is still a WIP. I have tests running locally but need to fix them up and push.~ Edit: Done

@dbrattli , interested to hear your thoughts. Would this be a collection type you'd be willing to merge?

brendanmaguire avatar Nov 11 '23 15:11 brendanmaguire

Looks very nice. Could we perhaps subclass Block to avoid so much code duplication? E.g by using the new Self type in block.py so functions return NonEmptyBlock instead of Block?

dbrattli avatar Nov 22 '23 12:11 dbrattli

Looks very nice. Could we perhaps subclass Block to avoid so much code duplication? E.g by using the new Self type in block.py so functions return NonEmptyBlock instead of Block?

Thanks for taking a look 👍

Some methods were not implemented in NonEmptyBlock as they wouldn't make sense. And some new ones were implemented. This change might involve pulling out the common part to an abstract BaseBlock class.

It might be a while before I get back to this PR again but I will take a look into that and see what's possible at some point.

brendanmaguire avatar Nov 23 '23 08:11 brendanmaguire

Hey @dbrattli . I just got around to looking at refactoring this to make NonEmptyBlock a subtype of Block. It seems it won't work because Self cannot be parameterized.

For example, if I change Block#collects signature to:

    def collect(self, mapping: Callable[[_TSource], Block[_TResult]]) -> Self[_TResult]:

I get the following error from pyright:

Expected no type arguments for class "Self"

brendanmaguire avatar Dec 17 '23 15:12 brendanmaguire