Skip to content

feat: add filter to match method#378

Merged
alaeddine-13 merged 5 commits into
mainfrom
feat-match-filter
Jun 1, 2022
Merged

feat: add filter to match method#378
alaeddine-13 merged 5 commits into
mainfrom
feat-match-filter

Conversation

@samsja

@samsja samsja commented May 30, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

This PR add the ability to do pre-filtering when calling match. Under the hood it passed the filter to the find function.

 doc.match(da, filter=filter)
  • code
  • tests
  • docs ( Not sure where to put it in the docs)

@codecov

codecov Bot commented May 30, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #378 (d19e40a) into main (1a3fb27) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #378      +/-   ##
==========================================
+ Coverage   86.20%   86.29%   +0.08%     
==========================================
  Files         134      134              
  Lines        6366     6368       +2     
==========================================
+ Hits         5488     5495       +7     
+ Misses        878      873       -5
Flag Coverage Δ
docarray 86.29% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
docarray/__init__.py 100.00% <100.00%> (ø)
docarray/array/mixins/match.py 75.00% <100.00%> (ø)
docarray/array/mixins/content.py 97.01% <0.00%> (-1.45%) ⬇️
docarray/helper.py 76.56% <0.00%> (-0.10%) ⬇️
docarray/array/mixins/io/pushpull.py 88.88% <0.00%> (+0.51%) ⬆️
docarray/document/mixins/helper.py 70.00% <0.00%> (+2.55%) ⬆️
docarray/document/mixins/sugar.py 68.00% <0.00%> (+16.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b80d393...d19e40a. Read the comment docs.

@samsja samsja requested review from JoanFM, alaeddine-13 and davidbp May 30, 2022 12:05
alaeddine-13
alaeddine-13 reviewed May 30, 2022
View reviewed changes

@alaeddine-13 alaeddine-13 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a few suggestions and it would be great if we update the docs

Comment thread docarray/array/mixins/match.py Outdated
Comment thread tests/unit/array/mixins/test_match.py Outdated
@alaeddine-13

alaeddine-13 commented May 30, 2022

Copy link
Copy Markdown
Member

Let's put docs in this section:
https://docarray.jina.ai/fundamentals/documentarray/matching/
basicly you can say that match and find support pre-filtering but it is specific to some backends and that you can check in the Document store section whether the backend supports pre-filtering or not. (also warn users that this is not supported in the in-memory backend)

samsja and others added 2 commits May 30, 2022 14:18
@samsja @alaeddine-13
fix: test function name fix
4265d2c 
Co-authored-by: AlaeddineAbdessalem <alaeddine-13@live.fr>
@samsja @alaeddine-13
fix: delete filtering mention in docstring
dd74b72 
Co-authored-by: AlaeddineAbdessalem <alaeddine-13@live.fr>
@samsja samsja mentioned this pull request May 30, 2022
Merged
@samsja samsja requested a review from alaeddine-13 May 31, 2022 15:46
JohannesMessner
JohannesMessner reviewed May 31, 2022
View reviewed changes
Comment thread docs/fundamentals/documentarray/matching.md Outdated
Comment thread docs/fundamentals/documentarray/matching.md Outdated
@samsja samsja linked an issue Jun 1, 2022 that may be closed by this pull request
Support pre-filtering in match function #385
Closed
@samsja @JohannesMessner
fix: apply johannes docs suggestion
d19e40a 
Co-authored-by: Johannes Messner <44071807+JohannesMessner@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 1, 2022

Copy link
Copy Markdown

📝 Docs are deployed on https://ft-feat-match-filter--jina-docs.netlify.app 🎉

davidbp
davidbp approved these changes Jun 1, 2022
View reviewed changes

@davidbp davidbp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@alaeddine-13 alaeddine-13 merged commit 464f24d into main Jun 1, 2022
@alaeddine-13 alaeddine-13 deleted the feat-match-filter branch June 1, 2022 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JohannesMessner JohannesMessner JohannesMessner left review comments

@alaeddine-13 alaeddine-13 alaeddine-13 approved these changes

@JoanFM JoanFM JoanFM approved these changes

+1 more reviewer

@davidbp davidbp davidbp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/core area/docs area/testing component/array size/m

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support pre-filtering in match function

5 participants

@samsja @alaeddine-13 @davidbp @JoanFM @JohannesMessner