Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maybe add a way to check if one item of a list of values exists #164

Closed
msiemens opened this issue Oct 12, 2017 · 8 comments
Closed

Maybe add a way to check if one item of a list of values exists #164

msiemens opened this issue Oct 12, 2017 · 8 comments

Comments

@msiemens
Copy link
Owner

Raised in https://forum.m-siemens.de/d/29-how-to-use-matches-with-int-values

Basically, with any and all we check, if a document's field – which is a list of values – contains a given value or only is a given value. From the docs:

>>> db.search(User.groups.any(['admin', 'sudo']))
[{'name': 'user2', 'groups': ['admin', 'user']},
 {'name': 'user3', 'groups': ['sudo', 'user']}]

>>> db.search(User.groups.all(['admin', 'user']))
[{'name': 'user2', 'groups': ['admin', 'user']}]

Note how in this case the field we check is a list. Now, right now we don't have a way for the reverse operation: if the field is a value, check if it's contained in a list. It could be something like this:

>>> db.search(User.name.in(['admin', 'user']))
[{'name': 'admin'}]
[{'name': 'user'}]

The issue is mainly naming. We can't name the method in as it's a Python keyword. I'm torn between calling it part_of or contained_in but both aren't exactly really intuitive when I think about it.


By the way, the workaround for now is using a custom test function:

>>> db.search(User.name.test(lambda name: name in ['admin', 'user']))
[{'name': 'admin'}]
[{'name': 'user'}]
@mlapierre
Copy link

How about one_of?

@eugene-eeo
Copy link
Contributor

My suggestion would be in_ – I've seen this convention used by some other database/query API but can't remember which one(s).

@msiemens
Copy link
Owner Author

msiemens commented Oct 18, 2017

@mlapierre @eugene-eeo Great suggestions! So a method could look like this:

db.search(User.name.one_of(['admin', 'user']))
# or
db.search(User.name.in_(['admin', 'user']))

To my eyes, the first option looks slightly better, but that's just my opinion. What do other folks think?

@alshapton
Copy link
Contributor

I'd vote for the latter since "in" seems to be more intuitive - is the User.name "in" a list of names ???

@msiemens
Copy link
Owner Author

My issue with in is that we can't use it directly as its a reserved keyword in Python. So we have to append an underscore or something similar (like @eugene-eeo's suggestion with in_) which to me dosn't look as elegant as one_of.

@alshapton
Copy link
Contributor

alshapton commented Oct 28, 2017

Faiir point re in. perhaps we should go with is_in. I think the reference to one in one_of may be confusing

@msiemens
Copy link
Owner Author

msiemens commented Nov 4, 2017

So we have three suggestions now:

# Variant 1:
db.search(User.name.one_of(['admin', 'user']))

# Variant 2:
db.search(User.name.in_(['admin', 'user']))

# Variant 3:
db.search(User.name.is_in(['admin', 'user']))

Personally I would lean towards one_of as it reads best to me (is the name one of admin or user?). It's a close call with is_in, but to me one_of wins because it quite nicely suggests that we have to search the list for a matching entry (so it's O(n) for lists, not O(1); of course depends on the data structure used).

It seems to me like one_of would do a better job of discouraging users to use something like age.one_of(range(1, 99)) than age.is_in(range(1, 99)). Anyways, it's a really close call and if there are no major objections, I'd implement this with one_of.

@alshapton
Copy link
Contributor

@msiemens On reflection (despite what I mentioned earlier in the thread) I think you are correct - one_of is probably the best option - go implement !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants