FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

refactor: do not recompute every time num_docs by JoanFM · Pull Request #1729 · docarray/docarray · GitHub

/ docarray Public

refactor: do not recompute every time num_docs - #1729

Merged
JoanFM merged 3 commits into
mainfrom
fix-num-docs
Jul 26, 2023
ConversationCommits (3)ChecksFiles changed
Merged

refactor: do not recompute every time num_docs#1729
JoanFM merged 3 commits into
mainfrom
fix-num-docs

Conversation

JoanFM commented Jul 26, 2023

Copy link
Copy Markdown
Member

No description provided.

JoanFM requested a review from jupyterjazz July 26, 2023 11:01
refactor: do not recompute every time num_docs
f95b623 
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>

codecov Bot commented Jul 26, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.27% 🎉

Comparison is base (24143a1) 83.21% compared to head (ca5cd2f) 84.48%.

❗ Current head ca5cd2f differs from pull request most recent head dbdc990. Consider uploading reports for the commit dbdc990 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1729      +/-   ##
==========================================
+ Coverage   83.21%   84.48%   +1.27%     
==========================================
  Files         134      134              
  Lines        8793     8793              
==========================================
+ Hits         7317     7429     +112     
+ Misses       1476     1364     -112
Flag Coverage Δ
docarray 84.48% <100.00%> (+1.27%) ⬆️

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

Files Changed Coverage Δ
docarray/index/backends/hnswlib.py 74.21% <100.00%> (ø)

... and 13 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jupyterjazz and others added 2 commits July 26, 2023 13:28
fix: k more than num docs
3ca5780 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
Merge branch 'main' of https://github.com/docarray/docarray into fix-…
dbdc990 
…num-docs

Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
jupyterjazz suggested changes Jul 26, 2023
View reviewed changes

jupyterjazz 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

value of k should not be more than number of vectors, otherwise lgtm

github-actions Bot commented Jul 26, 2023

Copy link
Copy Markdown

📝 Docs are deployed on https://ft-fix-num-docs--jina-docs.netlify.app 🎉

jupyterjazz approved these changes Jul 26, 2023
View reviewed changes

jupyterjazz 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I wasn't aware of this issue. Let's add 1-2 sentences in the code to explain why we're doing this

JoanFM merged commit 19aec21 into main Jul 26, 2023
jupyterjazz mentioned this pull request Aug 1, 2023
Closed
jupyterjazz mentioned this pull request Aug 23, 2023
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
jupyterjazz jupyterjazz approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/core size/s

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL