NotificationsYou must be signed in to change notification settings
Fork234
Star3.1k

Hotfix/optional leaf object#1865

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

Jump to bottom

Open

oytuntez wants to merge11 commits intodocarray:main

base:main

Choose a base branch

fromMotaWord:hotfix/optional-leaf-object

Open

Hotfix/optional leaf object#1865

oytuntez wants to merge11 commits intodocarray:mainfromMotaWord:hotfix/optional-leaf-object

Conversation

Copy link

oytuntez commentedFeb 14, 2024

This PR aims to improveDocList fields that areOptional or that are givenNone default value. Giving empty[] as default value would create entries in the subindex (with just the ID field, no other data), so I opted forNone.

The biggest issue that this PR aims to solve isDocList type fields would be required no matter if they have a default value or are marked asOptional.

Oytun Tez added5 commits

February 9, 2024 14:05

optional subfield improvements for BaseDocIndex

4c5592f

further type improvements

8c089dc

print logs

756deb8

removed debug prints

f9dca8d

remove unnecessary line change

f3e7fa7

JoanFM requested changes

Feb 14, 2024

View reviewed changes

Copy link

Member

JoanFM left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, is there a chance for you to add tests for the fixes? Also, I see code related to ES, does it belong to the same problem?

Copy link

codecovbot commentedFeb 14, 2024•
edited
Loading

Codecov Report

Attention:2 lines in your changes are missing coverage. Please review.

Comparison is base(7c1e18e) 84.68% compared to head(0a7e030) 84.67%.
Report is 2 commits behind head on main.

❗ Current head0a7e030 differs from pull request most recent head7555eb3. Consider uploading reports for the commit7555eb3 to get more accurate results

Files	Patch %	Lines
docarray/index/backends/elastic.py	66.66%	1 Missing⚠️
docarray/index/backends/hnswlib.py	75.00%	1 Missing⚠️

Additional details and impacted files

@@            Coverage Diff             @@##             main    #1865      +/-   ##==========================================- Coverage   84.68%   84.67%   -0.02%==========================================  Files         136      136                Lines        9261     9268       +7     ==========================================+ Hits         7843     7848       +5- Misses       1418     1420       +2