Skip to content

Add support to load multi page tiff files into chunks#845

Merged
samsja merged 10 commits intomainfrom
feat-support-multi-page-tiff-files
Nov 28, 2022
Merged

Add support to load multi page tiff files into chunks#845
samsja merged 10 commits intomainfrom
feat-support-multi-page-tiff-files

Conversation

@anna-charlotte
Copy link
Copy Markdown
Contributor

@anna-charlotte anna-charlotte commented Nov 24, 2022

A tiff file can contain multiple images. Currently, only the first image is being stored to a Documents .tensor when calling .load_uri_to_image_tensor(). We want to add support of multi-page tiff files now. In the case of a multi page tiff file, we want to save them to the Documents .chunks so that each image is wrapped in its own Document.

  • add support for multi page tiff files
  • check and update documentation, if required. See guide

@samsja
Copy link
Copy Markdown
Member

samsja commented Nov 24, 2022

this looks good. Should the method still be named .load_uri_to_image_tensor() ? Technically we don't put it into the tensor in your case

fix: add move channel to axis
5a04efc 
Signed-off-by: anna-charlotte <[email protected]>
@anna-charlotte anna-charlotte linked an issue Nov 24, 2022 that may be closed by this pull request
Support for tiff images #654
Closed
@anna-charlotte
Copy link
Copy Markdown
Contributor Author

anna-charlotte commented Nov 24, 2022

this looks good. Should the method still be named .load_uri_to_image_tensor() ? Technically we don't put it into the tensor in your case

Yes, I agree, this way the name doesn't really fit. Is changing the functions name not too much of a change though? Otherwise, I can change it to something like .load_uri_to_images

@samsja
Copy link
Copy Markdown
Member

samsja commented Nov 24, 2022

this looks good. Should the method still be named .load_uri_to_image_tensor() ? Technically we don't put it into the tensor in your case

Yes, I agree, this way the name doesn't really fit. Is changing the functions name not too much of a change though? Otherwise, I can change it to something like .load_uri_to_images

changing the function name is too big of a change. Maybe it is okay like this, Lets see if the test pass and we can rediscuss the API

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 24, 2022 •
edited
Loading

Codecov Report

Base: 86.06% // Head: 85.65% // Decreases project coverage by -0.41% ⚠️

Coverage data is based on head (a154027) compared to base (523dd61).
Patch coverage: 81.48% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #845      +/-   ##
==========================================
- Coverage   86.06%   85.65%   -0.42%     
==========================================
  Files         138      138              
  Lines        7046     7067      +21     
==========================================
- Hits         6064     6053      -11     
- Misses        982     1014      +32
Flag Coverage Δ
docarray 85.65% <81.48%> (-0.42%) ⬇️

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

Impacted Files Coverage Δ
docarray/document/mixins/image.py 63.10% <81.48%> (+2.86%) ⬆️
docarray/array/storage/sqlite/helper.py 75.00% <0.00%> (-13.89%) ⬇️
docarray/array/storage/base/seqlike.py 84.21% <0.00%> (-5.27%) ⬇️
docarray/array/storage/annlite/seqlike.py 88.00% <0.00%> (-4.00%) ⬇️
docarray/array/storage/sqlite/backend.py 82.50% <0.00%> (-3.75%) ⬇️
docarray/array/storage/annlite/backend.py 81.81% <0.00%> (-2.60%) ⬇️
docarray/array/storage/memory/backend.py 97.50% <0.00%> (-2.50%) ⬇️
docarray/array/storage/sqlite/seqlike.py 60.00% <0.00%> (-2.23%) ⬇️
docarray/array/storage/weaviate/backend.py 83.97% <0.00%> (-1.93%) ⬇️
docarray/array/storage/milvus/backend.py 91.35% <0.00%> (-1.86%) ⬇️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@anna-charlotte anna-charlotte marked this pull request as ready for review November 25, 2022 16:07
docs: update documentation
2591dc2 
Signed-off-by: anna-charlotte <[email protected]>
samsja
samsja reviewed Nov 28, 2022
View reviewed changes
alexcg1
alexcg1 suggested changes Nov 28, 2022
View reviewed changes
anna-charlotte and others added 2 commits November 28, 2022 13:14
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 28, 2022

📝 Docs are deployed on https://ft-feat-support-multi-page-tiff-files--jina-docs.netlify.app 🎉

alexcg1
alexcg1 approved these changes Nov 28, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@alexcg1 alexcg1 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@samsja samsja merged commit 8ceff1e into main Nov 28, 2022
@samsja samsja deleted the feat-support-multi-page-tiff-files branch November 28, 2022 14:31
@alexcg1 alexcg1 mentioned this pull request Dec 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samsja samsja samsja approved these changes

+1 more reviewer

@alexcg1 alexcg1 alexcg1 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/core area/docs area/testing component/document size/s

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for tiff images

4 participants

@anna-charlotte @samsja @codecov-commenter @alexcg1