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

Added fileObj to read file object #331

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

deepankurtaneja
Copy link

In Camelot there is already present functionality for reading PDF file using file path, but in order to read PDF files through file object was not possible in Camelot directly. Hence added one more function to allow read file object and modified few dependent functions to achieve it. This functionality has been tested on our side, please test it and merge it.

@vinayak-mehta vinayak-mehta self-requested a review May 26, 2019 10:59
@vinayak-mehta
Copy link
Contributor

I'll review this today.

@codecov-io
Copy link

Codecov Report

Merging #331 into master will decrease coverage by 3.15%.
The diff coverage is 22.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #331      +/-   ##
==========================================
- Coverage    87.2%   84.04%   -3.16%     
==========================================
  Files          13       13              
  Lines        1501     1561      +60     
  Branches      348      363      +15     
==========================================
+ Hits         1309     1312       +3     
- Misses        134      185      +51     
- Partials       58       64       +6
Impacted Files Coverage Δ
camelot/handlers.py 58.01% <24.61%> (-32.23%) ⬇️
camelot/io.py 60% <9.09%> (-40%) ⬇️

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 a1b85d2...725ff28. Read the comment docs.

Copy link
Contributor

@vinayak-mehta vinayak-mehta left a comment

Choose a reason for hiding this comment

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

@deepankurtaneja I'm not very keen on having another function called read_fileObj apart from read_pdf in the library's main interface. How does not having the ability to read a file object limit you? That will help me understand your use case better. If it makes sense to add that functionality, we can then think of a better interface.

@deepankurtaneja
Copy link
Author

@vinayak-mehta Actually I am trying to read a PDF file while I am uploading it through web application and there I was not able to use read_pdf() method for file object, hence I added the file object function so as to read the PDF while uploading it via web app.

@vinayak-mehta
Copy link
Contributor

Sorry for the delay in replies, please give me some time to look into this.

@GuillaumeKLECH
Copy link

Him i'm also interested in this feature, the need to have a local file (or a downloaded local file) prevents me to deploy camelot in a docker container without a volume.

Any updates about this PR ? Can i help ?

@yeus
Copy link

yeus commented Oct 5, 2020

Hi I would also really appreciate this feature... is there any progress in this task? From what I can see there are already two merge requests on this feature...

@yeus
Copy link

yeus commented Oct 5, 2020

just realizing this is a somewhat old? repository?

@deepankurtaneja if you read this, get the pull request over here:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants