Skip to content

added __getitem__ method#30

Closed
eugene-eeo wants to merge 1 commit intomsiemens:masterfrom
eugene-eeo:slices
Closed

added __getitem__ method#30
eugene-eeo wants to merge 1 commit intomsiemens:masterfrom
eugene-eeo:slices

Conversation

@eugene-eeo
Copy link
Contributor

Allows you to slice the tables:

table.insert_multiple([{'int': k} for k in range(1, 3)])

assert table[0] == [{'int': 1}]
assert table[1:2] == [{'int': 2}]

@msiemens
Copy link
Owner

msiemens commented Oct 5, 2014

With this PR db[...] is an alias for db.all()[...]. This makes sense as len(db) is a shorthand for len(db.all()). But I'm currently thinking about two things:

  1. The order of db.all(), which relies upon dict.values(), is not stable between modifications.

Keys and values are listed in an arbitrary order which is non-random, varies across Python implementations, and depends on the dictionary's history of insertions and deletions. (Python docs)

  1. We could as well make db[1] make a shorthand for db.get(id=1) and slicing (db[1:3]) respectively. But that would only work as long as our IDs are numeric and somewhat sorted.

I'd propose to don't add ambiguity and stick to typing a few characters more.

@eugene-eeo
Copy link
Contributor Author

What about implementing an index, ie db.index(*fields)? That way we can guarantee order.

@msiemens
Copy link
Owner

msiemens commented Oct 6, 2014

What would db.index(*fields) do?

@eugene-eeo
Copy link
Contributor Author

Using tuple comparison, indexes all of the documents within a table based on the given fields. For example a very simple case would be:

table.insert_multiple([{'x': 1, 'y': 1}, {'x': 1, 'y': 2}])
idx = table.index('x', 'y')
assert idx[0] == {'x': 1, 'y': 1}
assert idx[1] == {'x': 1, 'y': 2}

@msiemens
Copy link
Owner

msiemens commented Oct 7, 2014

I've had that idea, too. Maybe we could make that an extension?

idx = tinyindex(db, 'x', 'y')
...

@eugene-eeo
Copy link
Contributor Author

Sounds good to me!

@msiemens msiemens closed this Oct 10, 2014
@eugene-eeo
Copy link
Contributor Author

Implemented said extension: https://github.com/eugene-eeo/tinyindex

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.

2 participants