Skip to content

refactor: do not recompute every time num_docs#1729

Merged
JoanFM merged 3 commits intomainfrom
fix-num-docs
Jul 26, 2023
Merged

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

Conversation

@JoanFM
Copy link
Member

@JoanFM JoanFM commented Jul 26, 2023

No description provided.

@JoanFM 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 <[email protected]>
@codecov
Copy link

codecov bot commented Jul 26, 2023 •
edited
Loading

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
@jupyterjazz
fix: k more than num docs
3ca5780 
Signed-off-by: jupyterjazz <[email protected]>
Merge branch 'main' of https://github.com/docarray/docarray into fix-…
dbdc990 
…num-docs

Signed-off-by: Joan Fontanals Martinez <[email protected]>
jupyterjazz
jupyterjazz reviewed Jul 26, 2023
View reviewed changes
jupyterjazz
jupyterjazz suggested changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@jupyterjazz jupyterjazz left a comment

Choose a reason for hiding this comment

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

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

@github-actions
Copy link

github-actions bot commented Jul 26, 2023

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

jupyterjazz
jupyterjazz approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@jupyterjazz jupyterjazz left a comment

Choose a reason for hiding this comment

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

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

@JoanFM JoanFM merged commit 19aec21 into main Jul 26, 2023
@jupyterjazz jupyterjazz mentioned this pull request Aug 1, 2023
Closed
@jupyterjazz jupyterjazz mentioned this pull request Aug 23, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

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

@JoanFM @jupyterjazz