Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

[1LP][WIPTEST] Add match_items() and first() #8018

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[1LP][WIPTEST] Add match_items() and first() #8018

wants to merge 1 commit into from

Conversation

jarovo
Copy link
Contributor

@jarovo jarovo commented Oct 19, 2018

Purpose or Intent

Add some helper methods for filtering and picking first item.

@jarovo jarovo changed the title Add match_items() and first() [RFR] Add match_items() and first() Oct 23, 2018
@sshveta sshveta changed the title [RFR] Add match_items() and first() [1LP][RFR] Add match_items() and first() Oct 24, 2018


def match_items(items, filter):
''' Return only items matching filter dict. '''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double quotes on these docblocks please

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider making this a BaseCollection classmethod instead of just this standalone method, for clarity during use and ability to call self.match_items(self.all(), self.filters)



def first(items):
''' Return the first item in the iterable. '''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should definitely be a BaseCollection method, if its included at all. I'm hesitant to include it because its a single line wrapper that doesn't handle StopIteration to return None, or anything 'special' that would make it valuable beyond the caller just using next() or .pop() themselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is intended to be used when single item is expected to appear in some list or generator. So added value is that it handles both same way.

We can discuss adding handling StopIteration and raising some other exception instead, if you think it something we should do.

I've just realized I can be checking we got single and only value. So perhaps I should do that and rename this to singletone or only or something like that.

What do you think @mshriver

@mshriver mshriver changed the title [1LP][RFR] Add match_items() and first() [1LP][WIPTEST] Add match_items() and first() Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants