Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Support adding callables to path#445

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@rewritten
Copy link
Contributor

@rewrittenrewritten commentedDec 1, 2021
edited
Loading

Attempt to solve#423. The idea comes from lenses in general, and more specifically from Elixir'sAccess module and its kernel companion functionsget_in and friends.

query=Query().field.map(callable)==expectedquery=Query()['field'].map(callable)==expected

Maybe it would be even better to avoid an extra method altogether and use__getitem__:

query=Query().field[callable]==expectedquery=Query()['field'][callable]==expected

WDYT?

@rewritten
Copy link
ContributorAuthor

@msiemens do you think this solves the mapping problem? Do you want me to further improve test coverage / documentation / anything?

@msiemens
Copy link
Owner

Thanks@rewritten! This is an elegant implementation, I like it!

Maybe it would be even better to avoid an extra method altogether and use__getitem__:

I think it would be more consistent with how other parts of theQuery interface work to use the.map() function that you've already implemented 🙂


There's another question: How do we make sure thatlambdas passed to.map() have a consistent hash? For example:

>>>hash(lambda:1)275762436>>>hash(lambda:1)275850424

This would mean that a query that usesmap would create multiple entries in the query cache for the samelambda. A solution would be to build the hash from thelambdas code:

>>>hash((lambda:1).__code__)6824985394236043508>>>hash((lambda:1).__code__)6824985394236043508

But it gets even more complicated:lambdas can references local variables which are not included in the hash. Thus the query results would change but the query's hash would not and the query cache would return old results:

>>>x=2>>>hash((lambda:x).__code__)581326866784499933>>>x=20>>>hash((lambda:x).__code__)581326866784499933

This is something I don't have a solution for except to somehow exclude queries that usemap from the query cache…

@rewritten
Copy link
ContributorAuthor

Interesting. I probably would not worry about not being able to reuse a cached lambda for a query with the "same" lambda. On the contrary, rightly because even the same callable might have different effects depending on the state of the universe, I would explicitly avoid caching any callable at all.

x=1fn=lambday:y+xq=Query().val.map(fn)==4# q will return items with val==3x=2# now q will return other items

I'll look into the caching, but I'm quite sure that you don't want to cache the result of callable-based queries.

eugene-eeo reacted with thumbs up emoji

@rewritten
Copy link
ContributorAuthor

rewritten commentedDec 14, 2021
edited
Loading

Hey@msiemens I have given a try to mark queries as uncacheable when they contain a.map(callable) part, but it is not ideal, because:

  • all the query methods need to check whether the current query is cacheable, in order to set up cacheability of the returned query
  • all the test functions need to take it into account too

This last commit contains the needed changes, but I find it a bit inelegant. If you like it, I'm not opposed to merging, but I feel that there could be a better solution.

@rewritten
Copy link
ContributorAuthor

Hey@msiemens have you had an opportunity to see if this code fits your need?

@msiemens
Copy link
Owner

@rewritten I will have a closer look at your solution in the next two weeks, I'm just a little busy at the moment with the Christmas season 🙂 I'll check if I find a more elegant solution, but I also think that a pragmatic solution absolutely has its merits!

rewritten reacted with thumbs up emoji

@msiemensmsiemens merged commitf200118 intomsiemens:masterJan 10, 2022
@msiemens
Copy link
Owner

Thanks for your work! It's much appreciated 🙂

Regarding checking if the query is cachable or not: I think this implementation is fine. After all, explicit is better than implicit and all that 🙃

@msiemens
Copy link
Owner

I'll try to get a new version of TinyDB ready and published this weekend which will include your changes!

rewritten reacted with heart emoji

@msiemens
Copy link
Owner

This is now released in TinyDB v4.6.0 🥳

rewritten reacted with heart emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rewritten@msiemens

[8]ページ先頭

©2009-2025 Movatter.jp