Release camelot-fork 0.20.1 #353

foarsitter · 2023-02-09T10:14:00Z

Leaving this here so the it can be merged if there is any interest.

foarsitter · 2023-02-26T12:40:24Z

This pull-requests does a lot of things. Since things are getting back to life over here I will explain them a bit more.

The project is merged into cookiecutter-hypermodern-python which comes with a lot of features and documented workflows for i.e. how to release
It removes pdftopng as hard dependency since it is not compatible with python 3.9 and above. Tests that depend on poppler/pdftopng are skipped as long as pdftopng is not updated. The fork is compatible with 3.8 and above. It is up to discussion if we need to add more python versions.
It aligns the type of PDFHandler.filepath with PdfReader.stream so Path and IO objects are accepted. filepath can now be an in memory file upload for example.
I ran pre-commit to the whole branch to apply black & isort to the codebase.
The first pull-requests with bugfixes are merged: https://github.com/foarsitter/camelot/releases/tag/v0.20.1

From my point of view it makes sense to release version 1.0 as a stable release where we support ghostscript and do minor bugfixes and keep our dependencies up to date.

In version 2.0 we can add support for poppler/pdftopng.

What do you tink? Which changes can be cherry-picked and which changes need to dropped?

(cherry picked from commit 1632cfc)

…nd IO objects are accepted (cherry picked from commit 436998b)

Here Table.df is initialized as an empty DataFrame instead of None. This is for language servers to understand what members it might contain. For example, in the following line of code, ``` are_table_rows: List[bool] = [True] * table.df.shape[0] ``` Pyright complaints `Cannot access member "shape" for type "None"`. Such misunderstanding would not occur with this commit.

Cause: Empty horizontal and vertical dictionaries in t_bbox in method _text_bbox(t_bbox) Resolution: Added len check condition on t_bbox before updating (xmin, ymin, xmax, ymax) and initialised (xmin, ymin, xmax, ymax) to 0 if len is 0

changed the test name to be more aligned with other tests

ExSidius · 2023-03-07T18:35:34Z

@foarsitter @vinayak-mehta do you have a timeline around when this will be merged in?

I'm running into issues installing camelot on python 3.10 because of pdftopng.

Let me know if I can help at all.

foarsitter · 2023-03-07T18:52:56Z

@foarsitter @vinayak-mehta do you have a timeline around when this will be merged in?

I'm running into issues installing camelot on python 3.10 because of pdftopng.

Let me know if I can help at all.

Sorry, there is no timeline. If you need a solution asap install the fork (pip install camelot-fork).

MartinThoma · 2023-03-11T12:36:30Z

@foarsitter I would recommend to break a couple of changes out of this PR. That would have advantages:

It's more likely we get progress sooner
It's easier to review changes
Less merge conflicts

Especially black+isort would be VERY desirable to have in its own PR. That PR should not add / change anything else + it should document which commands were executed.

MartinThoma · 2023-03-11T12:42:58Z

@foarsitter #358 applies only black

README.md

vinayak-mehta · 2023-03-18T08:44:39Z

This is a big one, let me go through it this weekend

bosd

In general looks good to me. Few minor comments.

Only concern that this would possibly be incompatible with the multithreading.
But maybe thats a concern after we merged this.

.github/workflows/constraints.txt

pyproject.toml

bosd · 2023-07-15T10:47:15Z

Fixes #292 , #313, #329
Closes #269

.github/workflows/release.yml

LICENSE

MartinThoma

I want the license / copyright change to be explained / discussed.

foarsitter · 2023-07-15T21:13:46Z

Didn't know the cookiecutter template added a copyright with my name. Will remove it tomorrow. Thanks for checking.

foarsitter · 2023-07-16T08:03:15Z

Found some references at other places that needed to be updated. Sorry for the inconvenience.

MartinThoma

Thank you for the many improvements 🙏

I am not super familiar with camelot (and especially its CI). As a lot of files were touched (+4583 -975! and 145 files!) I recommend making some smoke-tests locally to ensure basic functionality.

In general, this would have been easier to review if the automatic changes (ruff? black? pyupgrade? isort?) were done in separate PRs.

foarsitter · 2023-07-16T17:40:50Z

The changes your are describing are in separated commits, you can select commits on the top left in the Files changed tab

What kind of smoke-tests do you have in mind? We are using the fork in production and all the tests of this PR are succeeding. Camelot has a pretty decent test coverage (88%) and a lot of different pdf types are tested: https://github.com/camelot-dev/camelot/tree/master/tests/files

One of the reasons I'm using the hypermodern cookiecutter template is that it has a lot of documentation. In example you can read about all the Github actions this PR includes over here and how to make a release over here.

I agree with you that the road I took is quite the spartan way but I think it is the fastest way forward.

MartinThoma · 2023-07-16T20:31:19Z

The changes your are describing are in separated commits

Interesting. Most of the time I don't look at single commits as they contain a lot of noise. As a maintainer of pypdf, I also don't like this model as it "packages" different unrelated changes. Hence it removes the freedom of choosing which of those I want to take in and which I don't want. The result is typically that it takes overall longer to merge those PRs + causes more work.

In the case of camelot I can understand it though. It's been a while since the last release. Please take my comment here more as a general wish. It's not meant as criticism in this specific case.

We are using the fork in production

Oh, nice! That should do it :-)

bosd · 2023-07-16T21:19:35Z

So.. concluding.. all lights are green ❤️ It's a go 😃 can we get a merge?

MartinThoma · 2023-07-17T06:04:36Z

Interesting, I thought you could merge @bosd .
@foarsitter Can you merge? Do you know who can?

Just to clarify: I can merge, but not release to PyPI. I'm a bit hesitant as I would rather like to support once in a while than become a core maintainer of another big project 😅

foarsitter · 2023-07-17T06:24:28Z

@MartinThoma yes I'm enable to merge. Since it is a large PR and a lot happened during last weekend I think it is appropriate to have a cooldown of a few days before merging so everyone can react.

Releasing to camelot on pypi is something we need to solve. In the meantime I can release to camelot-fork by forking this repo.

MartinThoma · 2023-07-17T16:10:31Z

Sounds good!

Regarding the releases, I've opened #389

bosd · 2023-07-17T16:36:47Z

Interesting, I thought you could merge @bosd .
Do you know who can?

No, I don't have any permissions here to merge.
I'm not listed as a maintainer (However I offered to be one).

As to my knowledge only @MartinThoma and @foarsitter are the only currently active maintainers.

foarsitter · 2023-07-26T11:22:23Z

Squash and merge is the only option for merging.... That need to be changed before merging.

MartinThoma · 2023-07-29T06:46:45Z

For this specific PR I agree. In general, I prefer squash-and-merge. PRs should only contain one change (once we are back on track)

bosd · 2023-08-04T05:15:59Z

Squash and merge is the only option for merging.... That need to be changed before merging.

So this means that dev is blocked/stalled again?? Or does any of you have the super powers to change this?

MartinThoma · 2023-08-04T05:48:06Z

I don't have access to the settings.

foarsitter · 2023-08-04T15:01:29Z

As long there is nobody around with full permission for GitHub and pypi this project could be considered dead.

MartinThoma · 2023-08-28T16:33:39Z

@vinayak-mehta Can you please give me full access so that I can adjust the project settings (temporarily enabling a merge-commit for this specific PR)?

MartinThoma · 2023-09-10T16:42:21Z

@foarsitter It seems as if I will not get the necessary permissions any time soon. Would it be ok for you if I squash-merge this PR and then make a release to PyPI?

foarsitter · 2023-09-10T18:17:06Z

Sure, no objection if its the only way forward. Go ahead :)

MartinThoma · 2023-09-10T21:46:44Z

https://github.com/camelot-dev/camelot/blob/master/.github/workflows/release.yml seems to have an issue:

MartinThoma · 2023-09-10T21:50:44Z

"Poetry could not find a pyproject.toml file in /home/runner/work/camelot/camelot or its parents"

I don't know why this part is failing.

However, the PyPI part should be removed as it's very likely not configured.

This PR contains a ton of improvements. They are merged via a squash-commit, but the individual changes are visible in the PR: * Add hypermodern python to the original repo * Add conftest.py and skip tests that depend on poppler * Align the type of PDFHandler.filepath with PdfReader.stream so Path and IO objects are accepted * Pre-commit * Initialized Table.df as empty DataFrame * Fixed: ValueError: min() arg is an empty sequence * Fix: unwanted data leaks into the last cell * added test case for method bbox_no_intersection method * Update test fixtures and remove unused imports * Fixed ZeroDivisionError in text_in_bbox * camelot-fork 0.20.1 * chore: revert changes to README.md * Remove references from pyproject to the fork * Fix install ghostscript * Poetry update * Removed references to me as person or to the camelot-fork repo --------- Co-authored-by: Jinjun Liang <[email protected]> Co-authored-by: Manish Patwari <[email protected]> Co-authored-by: zhy <[email protected]> Co-authored-by: Rahul.Bhave <[email protected]> Co-authored-by: Constantine Parkhimovich <[email protected]>

vinayak-mehta · 2024-12-25T19:51:55Z

Sorry for the dropping the ball big time here, I thought you both had all the permissions because you were members of the github organization, I didn't know github doesn't let you change minor repo level permissions even if you're an org member

foarsitter changed the title ~~Release~~ Release camelot-fork 0.20.1 Feb 9, 2023

foarsitter force-pushed the release branch from 8bdd80c to 197d316 Compare February 9, 2023 10:18

foarsitter and others added 11 commits February 26, 2023 13:42

Add hypermodern python to the original repo

260357f

Add conftest.py and skip tests that depend on poppler

753ecd5

(cherry picked from commit 1632cfc)

Align the type of PDFHandler.filepath with PdfReader.stream so Path a…

f7bcadf

…nd IO objects are accepted (cherry picked from commit 436998b)

Pre-commit

ad8cbb0

Fixed: ValueError: min() arg is an empty sequence

8717a01

Cause: Empty horizontal and vertical dictionaries in t_bbox in method _text_bbox(t_bbox) Resolution: Added len check condition on t_bbox before updating (xmin, ymin, xmax, ymax) and initialised (xmin, ymin, xmax, ymax) to 0 if len is 0

Fix: unwanted data leaks into the last cell

dcc78f4

added test case for method bbox_no_intersection method

36dce51

changed the test name to be more aligned with other tests

Update test fixtures and remove unused imports

c24be55

Fixed ZeroDivisionError in text_in_bbox

252c59b

camelot-fork 0.20.1

3d6c019

foarsitter force-pushed the release branch from 197d316 to 3d6c019 Compare February 26, 2023 12:44

MartinThoma mentioned this pull request Mar 11, 2023

[WIP] Add support for parsing PDF pages in parallel #237

Closed

4 tasks

MartinThoma reviewed Mar 11, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

chore: revert changes to README.md

8dba0e7

bosd mentioned this pull request Jul 14, 2023

pdftopng installation issues #380

Open

foarsitter mentioned this pull request Jul 15, 2023

STY: Apply black #358

Closed

bosd requested changes Jul 15, 2023

View reviewed changes

.github/workflows/constraints.txt Show resolved Hide resolved

pyproject.toml Outdated Show resolved Hide resolved

foarsitter added 3 commits July 15, 2023 13:02

Remove references from pyproject to the fork

9af09b5

Fix install ghostscript

39a4f94

Merge upstream master into fork

331f301

MartinThoma reviewed Jul 15, 2023

View reviewed changes

.github/workflows/release.yml Show resolved Hide resolved

MartinThoma reviewed Jul 15, 2023

View reviewed changes

LICENSE Outdated Show resolved Hide resolved

MartinThoma requested changes Jul 15, 2023

View reviewed changes

Removed references to me as person or to the camelot-fork repo

3938222

MartinThoma approved these changes Jul 16, 2023

View reviewed changes

MartinThoma mentioned this pull request Sep 2, 2023

We need more maintainers #343

Open

MartinThoma merged commit e12fba4 into camelot-dev:master Sep 10, 2023

foarsitter mentioned this pull request Sep 24, 2023

Add pypdfium2 rendering backend #384

Closed

MartinThoma mentioned this pull request Feb 25, 2024

Merge open PRs into pypdf_table_extraction py-pdf/pypdf_table_extraction#1

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release camelot-fork 0.20.1 #353

Release camelot-fork 0.20.1 #353

foarsitter commented Feb 9, 2023 •

edited

Loading

foarsitter commented Feb 26, 2023

ExSidius commented Mar 7, 2023

foarsitter commented Mar 7, 2023

MartinThoma commented Mar 11, 2023

MartinThoma commented Mar 11, 2023

vinayak-mehta commented Mar 18, 2023

bosd left a comment

bosd commented Jul 15, 2023 •

edited

Loading

MartinThoma left a comment

foarsitter commented Jul 15, 2023

foarsitter commented Jul 16, 2023

MartinThoma left a comment

foarsitter commented Jul 16, 2023

MartinThoma commented Jul 16, 2023

bosd commented Jul 16, 2023

MartinThoma commented Jul 17, 2023

foarsitter commented Jul 17, 2023

MartinThoma commented Jul 17, 2023

bosd commented Jul 17, 2023

foarsitter commented Jul 26, 2023

MartinThoma commented Jul 29, 2023

bosd commented Aug 4, 2023

MartinThoma commented Aug 4, 2023

foarsitter commented Aug 4, 2023

MartinThoma commented Aug 28, 2023

MartinThoma commented Sep 10, 2023

foarsitter commented Sep 10, 2023

MartinThoma commented Sep 10, 2023

MartinThoma commented Sep 10, 2023

vinayak-mehta commented Dec 25, 2024

Release camelot-fork 0.20.1 #353

Release camelot-fork 0.20.1 #353

Conversation

foarsitter commented Feb 9, 2023 • edited Loading

foarsitter commented Feb 26, 2023

ExSidius commented Mar 7, 2023

foarsitter commented Mar 7, 2023

MartinThoma commented Mar 11, 2023

MartinThoma commented Mar 11, 2023

vinayak-mehta commented Mar 18, 2023

bosd left a comment

Choose a reason for hiding this comment

bosd commented Jul 15, 2023 • edited Loading

MartinThoma left a comment

Choose a reason for hiding this comment

foarsitter commented Jul 15, 2023

foarsitter commented Jul 16, 2023

MartinThoma left a comment

Choose a reason for hiding this comment

foarsitter commented Jul 16, 2023

MartinThoma commented Jul 16, 2023

bosd commented Jul 16, 2023

MartinThoma commented Jul 17, 2023

foarsitter commented Jul 17, 2023

MartinThoma commented Jul 17, 2023

bosd commented Jul 17, 2023

foarsitter commented Jul 26, 2023

MartinThoma commented Jul 29, 2023

bosd commented Aug 4, 2023

MartinThoma commented Aug 4, 2023

foarsitter commented Aug 4, 2023

MartinThoma commented Aug 28, 2023

MartinThoma commented Sep 10, 2023

foarsitter commented Sep 10, 2023

MartinThoma commented Sep 10, 2023

MartinThoma commented Sep 10, 2023

vinayak-mehta commented Dec 25, 2024

foarsitter commented Feb 9, 2023 •

edited

Loading

bosd commented Jul 15, 2023 •

edited

Loading