Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: return iterable on @ selector #143

Closed
wants to merge 2 commits into from

Conversation

alaeddine-13
Copy link
Member

Do not merge
This PR is only for testing purposes

@codecov
Copy link

codecov bot commented Feb 24, 2022

Codecov Report

Merging #143 (fa3387c) into main (e5a4ed5) will decrease coverage by 12.15%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #143       +/-   ##
===========================================
- Coverage   83.75%   71.60%   -12.16%     
===========================================
  Files         108      108               
  Lines        4618     4617        -1     
===========================================
- Hits         3868     3306      -562     
- Misses        750     1311      +561     
Flag Coverage Δ
docarray 71.60% <66.66%> (-12.16%) ⬇️

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

Impacted Files Coverage Δ
docarray/array/mixins/traverse.py 42.25% <50.00%> (-52.20%) ⬇️
docarray/array/mixins/find.py 83.82% <100.00%> (-1.48%) ⬇️
docarray/math/distance/paddle.py 0.00% <0.00%> (-66.67%) ⬇️
docarray/array/mixins/embed.py 13.69% <0.00%> (-63.02%) ⬇️
docarray/math/distance/tensorflow.py 0.00% <0.00%> (-61.91%) ⬇️
docarray/array/mixins/plot.py 8.42% <0.00%> (-56.32%) ⬇️
docarray/array/mixins/text.py 50.00% <0.00%> (-50.00%) ⬇️
docarray/math/distance/torch.py 0.00% <0.00%> (-50.00%) ⬇️
docarray/array/storage/memory/find.py 41.81% <0.00%> (-49.10%) ⬇️
... and 34 more

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 e5a4ed5...fa3387c. Read the comment docs.

Copy link
Member

@JoanFM JoanFM left a comment

Choose a reason for hiding this comment

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

I would suggest then to create an abstract class:

class DocumentIterable(Iterable[Document]):
# The R DocumentArray 
...

class DocumentArray(DocumentIterable):
 # THE RW DOCUMENTARRAY
# DocumentArray implements DocumentIterable and many more things 

that implements all the methods that do not to rely on the uniqueness of IDS. Like this there will be much better clearer of what is what Plus many of the accessors could be shared.

One could say that `Document

@hanxiao hanxiao changed the title feat: return iterable on traverse_flat feat: return iterable on @ selector Mar 10, 2022
@hanxiao hanxiao mentioned this pull request Mar 10, 2022
21 tasks
@hanxiao
Copy link
Member

hanxiao commented Mar 31, 2022

close due to #239

@hanxiao hanxiao closed this Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants