6
\$\begingroup\$

This function takes a file path to a PDF file, and a search string(s). It spits out a count of the number of times the string(s) shows in the PDF. Any ideas how I can make it faster?

It can be tested with any PDF (not password protected.)

I frequently use this to scan a list of words in a large number of PDFs and it is slow!

import os
import fitz # using PyMuPDF
import string
def count_of_string_in_pdf_file_advanced_alt(next_pdfs_path: str, search_string: str) -> int:    
    """
    count the number of times a string is in a PDF file. Case insensitive.
            Parameters:
                    next_pdfs_path (str): file path to a PDF file
                    search_string (str): string to look for/count in PDF file
            Returns:
                    count (int): a count of the number of occurrences
    """
    count = 0
    flag = True
    search_string = str(search_string.lower().strip()) 
    if os.path.isfile(next_pdfs_path):      # check file is a real file/filepath
        try:
            text = ''
            with fitz.open(next_pdfs_path) as doc:
                for page in doc:
                    text += page.get_text()
            while flag:
                text = text.translate(str.maketrans('', '', string.punctuation)) # remove puncuation
                words = text.lower().split() # cleanup and split the text into list of words
                words = sorted(words)
                if search_string in words:
                    count = words.count(search_string)
                    break
                else:
                    flag = False
                # return count
        except (RuntimeError, IOError):
            pass
    return count

I profiled it using a single string and this 74 page PDF https://thedocs.worldbank.org/en/doc/79bb914488308f1e75744fccc4e12cb3-0290032021/world-bank-notes-on-debarred-firms-and-individuals-pdf

                 7998 function calls (7989 primitive calls) in 0.182 seconds

   Ordered by: call count

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
      453    0.000    0.000    0.000    0.000 {built-in method builtins.getattr}
      444    0.000    0.000    0.000    0.000 fitz.py:621(__getitem__)
      312    0.000    0.000    0.000    0.000 {built-in method builtins.len}
      300    0.000    0.000    0.000    0.000 {method 'own' of 'SwigPyObject' objects}
      296    0.000    0.000    0.000    0.000 fitz.py:3003(CheckParent)
      296    0.000    0.000    0.000    0.000 fitz.py:3047(<lambda>)
      296    0.000    0.000    0.000    0.000 {built-in method builtins.abs}
      151    0.000    0.000    0.000    0.000 {method 'items' of 'dict' objects}
      149    0.000    0.000    0.000    0.000 fitz.py:5755(__contains__)
      149    0.000    0.000    0.000    0.000 fitz.py:4408(page_count)
      149    0.000    0.000    0.000    0.000 {built-in method fitz._fitz.Document_page_count}
      148    0.000    0.000    0.001    0.000 weakref.py:164(__setitem__)
      148    0.000    0.000    0.000    0.000 weakref.py:347(__new__)
      148    0.000    0.000    0.000    0.000 {built-in method __new__ of type object at 0x00007FFF295E13D0}
      148    0.000    0.000    0.000    0.000 {built-in method _weakref.proxy}
      148    0.000    0.000    0.000    0.000 {built-in method builtins.id}
      148    0.000    0.000    0.000    0.000 {built-in method builtins.round}
       84    0.000    0.000    0.000    0.000 {built-in method builtins.hasattr}
       77    0.000    0.000    0.000    0.000 weakref.py:252(popitem)
       77    0.000    0.000    0.001    0.000 weakref.py:243(values)
       77    0.000    0.000    0.000    0.000 {method 'popitem' of 'dict' objects}
       77    0.001    0.000    0.001    0.000 {method 'lower' of 'str' objects}
       76    0.000    0.000    0.000    0.000 _weakrefset.py:17(__init__)
       76    0.000    0.000    0.000    0.000 _weakrefset.py:27(__exit__)
       76    0.000    0.000    0.000    0.000 weakref.py:121(_commit_removals)
       76    0.000    0.000    0.000    0.000 _weakrefset.py:21(__enter__)
       76    0.000    0.000    0.000    0.000 <frozen _collections_abc>:966(clear)
       76    0.000    0.000    0.000    0.000 {method 'values' of 'dict' objects}
       76    0.000    0.000    0.000    0.000 {method 'pop' of 'list' objects}
       76    0.000    0.000    0.000    0.000 {method 'add' of 'set' objects}
       76    0.000    0.000    0.000    0.000 {method 'remove' of 'set' objects}
       75    0.000    0.000    0.000    0.000 weakref.py:289(update)
       75    0.000    0.000    0.000    0.000 weakref.py:104(__init__)
       75    0.000    0.000    0.001    0.000 fitz.py:7227(_reset_annot_refs)
       75    0.000    0.000    0.004    0.000 fitz.py:5778(__getitem__)
       75    0.000    0.000    0.002    0.000 fitz.py:7240(_erase)
       74    0.000    0.000    0.000    0.000 weakref.py:352(__init__)
       74    0.000    0.000    0.000    0.000 weakref.py:152(__contains__)
       74    0.000    0.000    0.000    0.000 fitz.py:497(__init__)
       74    0.000    0.000    0.004    0.000 fitz.py:4146(load_page)
       74    0.000    0.000    0.000    0.000 fitz.py:434(util_make_rect)
       74    0.001    0.000    0.135    0.002 utils.py:753(get_text)
       74    0.000    0.000    0.126    0.002 fitz.py:6009(get_textpage)
       74    0.000    0.000    0.000    0.000 fitz.py:926(__init__)
       74    0.000    0.000    0.000    0.000 fitz.py:5898(<lambda>)
       74    0.000    0.000    0.000    0.000 fitz.py:636(__len__)
       74    0.000    0.000    0.001    0.000 fitz.py:3046(JM_TUPLE3)
       74    0.000    0.000    0.001    0.000 fitz.py:6892(cropbox)
       74    0.000    0.000    0.125    0.002 fitz.py:6002(_get_textpage)
       74    0.000    0.000    0.000    0.000 fitz.py:6972(rotation)
       74    0.000    0.000    0.000    0.000 fitz.py:5839(_forget_page)
       74    0.000    0.000    0.005    0.000 fitz.py:9002(extractText)
       74    0.000    0.000    0.000    0.000 fitz.py:8909(<lambda>)
       74    0.000    0.000    0.002    0.000 fitz.py:7253(__del__)
       74    0.000    0.000    0.005    0.000 fitz.py:8993(_extractText)
       74    0.000    0.000    0.002    0.000 fitz.py:9086(__del__)
       74    0.000    0.000    0.000    0.000 {built-in method fitz._fitz.util_make_rect}
       74    0.003    0.000    0.003    0.000 {built-in method fitz._fitz.Document_load_page}
       74    0.000    0.000    0.000    0.000 {built-in method fitz._fitz.delete_Page}
       74    0.129    0.002    0.129    0.002 {built-in method fitz._fitz.Page__get_textpage}

Some things I have tried

# I tested using join rather than concatenating but found it to be slower.
        # for page in doc:
        #     text += page.get_text()
        text = ' '.join(page.get_text() for page in doc)
\$\endgroup\$
3
  • 1
    \$\begingroup\$ (When trying to speed up something, please invest at least 5 seconds of profile.)(A trailing codereview code block needs to be followed by a newline.) \$\endgroup\$
    – greybeard
    Jan 30 at 19:30
  • \$\begingroup\$ If you dig into the fitz module's get_text() function, you actually have options of getting a dict of the full output, which contains massive amounts of extra data. Not useful for you in this case, but reading the PDF is non-trivially expensive due to the format. \$\endgroup\$
    – Nelson
    Jan 31 at 3:56
  • 1
    \$\begingroup\$ I have rolled back Rev 3 → 2. Please see What should I do when someone answers my question?. \$\endgroup\$ Jan 31 at 18:12

2 Answers 2

10
\$\begingroup\$

where did the time go?

Thank you for including measurements.

Ordering the profiler output by call count is uninteresting. What you care about is time spent, and the output makes it pretty clear that nearly all the delay happens in get_text(). Everything else is just noise.

Any ideas how I can make it faster?

If you're not happy with the current PDF text extractor, then choose another. It might come with convenient python bindings. Or it might just produce plain text on stdout, which a parent python process can consume and grep through.

Consider writing a foo.txt cache file each time you parse some original foo.pdf document. Then you will never have to parse a document twice, and elapsed time on subsequent runs boils down to time for grep'ing a text file. This is a time vs. space tradeoff, where spending disk blocks saves us CPU seconds.

design of API

frequently use this to scan a list of words

The current Public API accepts a single word and reports a count. If your list of N words starts with "apple", "banana", the call sequence would be

pdf = "/some/where/World-Bank-Notes-on-Debarred-Firms-and-Individuals.pdf"
a = count_of_string_in_pdf_file_advanced_alt(pdf, "apple")
b = count_of_string_in_pdf_file_advanced_alt(pdf, "banana")
...

You just did N expensive get_text operations followed by a trivial cost grep-and-count operation.

Rather than a single word at a time, prefer to pass in a container of words:

>>> from collections import Counter 
>>> cnt = Counter(dict(apple=0, banana=0))
>>> cnt
Counter({'apple': 0, 'banana': 0})

That way for each .PDF file you can get_text just once, and report all the desired word counts as a single return value.

An alternate approach would be to pass in a regex:

pattern = re.compile(r'\b(apple|banana)\b', re.I)

long identifiers

def count_of_string_in_pdf_file_advanced_alt(next_pdfs_path, ...

These would probably benefit from being shortened a little. If you still have more things to say about them, the very nice existing docstring is a good place to do that.

short identifier

            while flag:

That is too vague.

Yes, we can see it is of type bool. But we don't know what it means.

It should be called not_found.

Or better, invert its meaning and rephrase, since humans do better with positive identifiers.

            while not found:

It's unclear why we're even looping here at all. The if will either break or change the flag. Both of those actions cause the loop body's search to happen exactly once. Maybe this is leftover debug from when you were exploring other ways of looping over pages?

ETAF rather than LBYL

    if os.path.isfile(next_pdfs_path):      # check file is a real file/filepath

It's unclear why this check is needed. If fitz.open() raises, it raises, no big deal. And then either caller would see the exception bubble up the stack, or caller would get a zero count, however you prefer to handle that.

Prefer Path() over str when putting pathname parameters in a function signature.

BTW the text.translate() is lovely, it runs very fast on the inside. You have an opportunity to call str.maketrans() just once when this module is imported, and then keep re-using that translation table, but that would be a very tiny savings in the scheme of things.

Rather than deleting punctuation, you might prefer to substitute such characters with SPACE. My concern is that text like "spin-weave" is being turned into the single word "spinweave", where likely "spin weave" would be preferable.

unneeded action

                words = sorted(words)

This looks like a setup for calling groupby() to obtain counts of every word in the document. But then we go on to do something else. Maybe it is leftover debug?

This is a relatively inexpensive operation, but there's no need to do it so we may as well drop it.


This codebase achieves many of its design goals. Apparently its future development will mostly be about choosing and integrating with faster PDF libraries.

I would be willing to delegate or accept maintenance tasks on this code.

\$\endgroup\$
3
  • \$\begingroup\$ "Consider writing a foo.txt cache file" how did I not think of doing that before now! \$\endgroup\$
    – Cam
    Jan 30 at 17:34
  • \$\begingroup\$ "Rather than a single word at a time, prefer to pass in a container of words:" yes I had considered doing this. Will give it a try. \$\endgroup\$
    – Cam
    Jan 30 at 17:38
  • 4
    \$\begingroup\$ For others, ETAF="Easier To Ask Forgiveness (rather than permission)"-> just run it, handle exceptions later, LBYL="Look Before You Leap" -> check pre-conditions first so we don't need to handle exceptions. \$\endgroup\$
    – justhalf
    Jan 31 at 8:53
2
\$\begingroup\$

So I have taken onboard most of what was suggested and have moved towards pushing a list into the function. I have cleaned up the function, and am now using collections.counter which seems to be blisteringly fast when working with a large list but slower when just a single string.

testing

# Generate 1 million short strings

from random import choice
from string import ascii_lowercase, digits
chars = ascii_lowercase + digits
lst = [''.join(choice(chars) for _ in range(5)) for _ in range(1000000)]

# function (yes still with the verbose name)

import os
import fitz
import string
from collections import Counter
def count_of_string_in_pdf_file_advanced_alt2(next_pdfs_path: str, search_list: list) -> int:    
    """
    count the number of times a string is in a PDF file. Case insensitive.
            Parameters:
                    next_pdfs_path (str): file path to a PDF file
                    search_list (list): list of strings to look for/count in PDF file
            Returns:
                    count (dict): a count of the number of occurrences of each string
    """
    search_list = [str(word.lower().strip()) for word in search_list]
    try:
        text = ''
        with fitz.open(next_pdfs_path) as doc:      # using PyMuPDF
            for page in doc:
                text += page.get_text()

        text = text.translate(str.maketrans('', '', string.punctuation)) # remove puncuation
        words = text.lower().split() # cleanup and split the text into list of words
        word_count = Counter(words)
        # make new dict keeping only the required strings
        word_count = {k: word_count[k] for k in search_list}
    except (RuntimeError, IOError):
        pass
    return word_count
 

Profile

import cProfile
import pstats
cProfile.run("count_of_string_in_pdf_file_advanced_alt(file_path, lst)", 'profile_results')
stats = pstats.Stats('profile_results')
stats.sort_stats('time').print_stats()

output with 1 million strings:-
3002833 function calls (3002829 primitive calls) in 1.442 seconds

output with 1 string
2800 function calls (2798 primitive calls) in 0.410 seconds

So with counting a single string it is slower than using count, but it scales very well!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.