Message when calling len() on LazyFilter #817
Conversation
| def from_features(features: Union[Iterable[Features], LazyMixin]) -> "FeatureSet": | ||
| return ( | ||
| FeatureSet([f for f in features]) | ||
| if isinstance(features, LazyMixin) |
There was a problem hiding this comment.
Curious why not use one of these two options for both cases? I recall list has some weird behavior with regard to whether len is implemented, but maybe we can just always use comprehension?
There was a problem hiding this comment.
Yeah, list() requires __len__ to be implemented, which is why I had to use list comprehension. However, it seems like list comprehension may be slower than list() (see this), which is why I didn't make it the default.
| def __add__(self, other) -> "LazyIteratorChain": | ||
| return LazyIteratorChain(self, other) | ||
|
|
||
| def __len__(self) -> int: | ||
| raise NotImplementedError( |
There was a problem hiding this comment.
Can you check other lazy iterator classes and see if they can also use this error message? At least one that comes to my mind is LazyFlatten.
There was a problem hiding this comment.
Added to LazyFlattener. All other lazy iterator classes already have __len__ defined.
Currently, when
len()is called on an object of typeLazyFilter(which is the case when a filter is applied on a lazily loaded manifest), it will throw aTypeError:After this change, we instead show an informative message: