search

jsvine / pdfplumber

Plumb a PDF for detailed information about each char, rectangle, line, et cetera — and easily extract text and tables.
MIT License
6.48k stars 658 forks source link

mixed documentation of PDF.close() and Page.flush_cache() in README #1042

Closed luketudge closed 11 months ago

luketudge commented 11 months ago

Something is not right about the main README and perhaps needs updating.

The explanation of PDF.close() here mentions clearing the cache for Page objects. But calling PDF.close() does not clear the cache for the individual pages. To do that, we need Page.flush_cache(). This part of the README also suggests that .flush_cache() is deprecated in favor of .close(), but Page objects do not have a .close() method.

Maybe the documentation of PDF and Page has gotten mixed up? Clearer would be to explain here what .close() does for the PDF object, and to add documentation of cache clearing for Page objects to a different section.

jsvine commented 11 months ago

Thanks for flagging this, @luketudge! It does look like things got a bit mixed up over the course of some changes. I'm proposing fixing the issue via this commit: https://github.com/jsvine/pdfplumber/commit/ba58e16450f86066a1fe9aceaccc0c2e7b262a14

It (re-)adds the missing Page.close() method, has PDF.close() close all pages, and adjusts the documentation. Does this seem right to you? Or still a bit confusing/wrong?

luketudge commented 11 months ago

@jsvine Great! This looks exactly right. Thanks for picking it up so quickly.

jsvine commented 11 months ago

Super! Closing this as fixed via https://github.com/jsvine/pdfplumber/commit/ba58e16450f86066a1fe9aceaccc0c2e7b262a14