8
\$\begingroup\$

Take a resume (or “CV” outside of the US) and return all the text within it in a formatted way. Right now the below script outputs it to a txt file. I used this guide as inspiration and have looked to improve on it, by using slightly saner control flow and functions. Although the script works as intended, there are quite a lot of things that smell very bad.

Bad Things:

  1. Horrendous main()
  2. Kind of crazy looping (lots of nested indentations which is usually a bad sign).
  3. 2D structures and LOTS of lists (again! A bad sign).
  4. No use of yield so materialising a lot in memory.
  5. No use of @dataclass/NamedTuple (I feel like I should be modelling the PDFPage at least).
  6. Could this be vectorised?
  7. Converting it to an object-oriented design seems like a OK idea.
  8. Dumb statements like using pass and if table_in_page == -1
  9. PEP8 Violations

I am lacking creativity to get this to an elegant solution and thought I would add it here to see if there are any fresh minds that want to rework it.

The Code

from typing import Any, Optional

import pdfplumber
from pdfminer.high_level import extract_pages
from pdfminer.layout import LTPage, LTTextContainer, LTChar


def text_extraction(element: LTTextContainer) -> tuple[str, list[str]]:
    """
    Extracts text and unique formats (font names and sizes) from a given element.

    Parameters:
        element (LTTextContainer): The element from which text and formats are extracted.

    Returns:
        tuple[str, list[str]]: A tuple containing the extracted text and a list of unique formats.
    """
    line_text = element.get_text()
    line_formats = set()

    for text_line in element:
        if isinstance(text_line, LTTextContainer):
            for character in text_line:
                if isinstance(character, LTChar):
                    format_info = f"{character.fontname}, {character.size}"
                    line_formats.add(format_info)

    format_per_line = list(line_formats)
    return line_text, format_per_line


def extract_table(pdf_path: str, page_num: int, table_num: int) -> Optional[list[list[str]]]:
    """
    Extracts a specified table from a given page of a PDF document.

    Parameters:
        pdf_path (str): The file path of the PDF document.
        page_num (int): The page number from which to extract the table.
        table_num (int): The index of the table on the page to extract.

    Returns:
        Optional[list[list[str]]]: A 2D list representing the extracted table, or None if an error occurs.
    """
    try:
        with pdfplumber.open(pdf_path) as pdf:
            # Check if the page number is valid
            if page_num < 0 or page_num >= len(pdf.pages):
                raise ValueError("Page number out of range.")

            table_page = pdf.pages[page_num]
            tables = table_page.extract_tables()

            # Check if the table number is valid
            if table_num < 0 or table_num >= len(tables):
                raise ValueError("Table number out of range.")

            return tables[table_num]
    except Exception as e:
        print(f"An error occurred: {e}")
        return None


def table_converter(table: list[list[str]]) -> str:
    """
    Converts a 2D table into a string format, where each cell is separated by '|'
    and each row is on a new line. Newline characters in cells are replaced with spaces,
    and None values are converted to the string 'None'.

    Parameters:
        table (list[list[str]]): The 2D table to convert.

    Returns:
        str: The string representation of the table.

    Example usage:
        table = [['Name', 'Age'], ['Alice', '23'], ['Bob', None]]
        print(table_converter(table))
    """
    converted_rows = []
    for row in table:
        cleaned_row = [
            item.replace('\n', ' ') if item is not None else 'None'
            for item in row
        ]
        converted_rows.append('|' + '|'.join(cleaned_row) + '|')

    return '\n'.join(converted_rows)


def is_element_inside_any_table(element, page: LTPage, tables: list[Any]) -> bool:
    """
    Checks whether a given element is inside any of the tables on a PDF page.

    Parameters:
        element: The element to check.
        page (LTPage): The PDF page.
        tables (List[Any]): A list of tables, where each table is an object with a bounding box.

    Returns:
        bool: True if the element is inside any of the tables, False otherwise.
    """
    x0, y0up, x1, y1up = element.bbox
    page_height = page.bbox[3]
    # Transform coordinates
    y0, y1 = page_height - y1up, page_height - y0up

    for table in tables:
        tx0, ty0, tx1, ty1 = table.bbox
        # Check if element bbox is inside table bbox
        if tx0 <= x0 < x1 <= tx1 and ty0 <= y0 < y1 <= ty1:
            return True

    return False


def find_table_for_element(element, page: LTPage, tables: list[Any]) -> Optional[int]:
    """
    Finds the index of the table that a given element is inside on a PDF page.

    Parameters:
        element: The element to check.
        page (LTPage): The PDF page.
        tables (list[Any]): A list of tables, where each table is an object with a bounding box.

    Returns:
        Optional[int]: The index of the table that contains the element, or None if not found.
    """
    x0, y0up, x1, y1up = element.bbox
    page_height = page.bbox[3]
    # Transform coordinates
    y0, y1 = page_height - y1up, page_height - y0up

    for i, table in enumerate(tables):
        tx0, ty0, tx1, ty1 = table.bbox
        if tx0 <= x0 < x1 <= tx1 and ty0 <= y0 < y1 <= ty1:
            return i  # Return the index of the table

    return None


def process_tables(tables, pdf_path, pagenum, text_from_tables):
    # Extracting the tables of the page
    for table_num in range(len(tables)):
        # Extract the information of the table
        table = extract_table(pdf_path, pagenum, table_num)
        # Convert the table information in structured string format
        table_string = table_converter(table)
        # Append the table string into a list
        text_from_tables.append(table_string)


def process_text_element(element, page_text, line_format, page_content):
    # Check if the element is text element
    if isinstance(element, LTTextContainer):
        # Use the function to extract the text and format for each text element
        (line_text, format_per_line) = text_extraction(element)
        # Append the text of each line to the page text
        page_text.append(line_text)
        # Append the format for each line containing text
        line_format.append(format_per_line)
        page_content.append(line_text)

    return line_format, page_content


def main(filepath: str) -> None:
    pdf = open(filepath, 'rb')
    text_per_page = {}

    # We extract the pages from the PDF
    for pagenum, page in enumerate(extract_pages(filepath)):

        # Initialize the variables needed for the text extraction from the page
        page_text = []
        line_format = []
        text_from_images = []
        text_from_tables = []
        page_content = []

        # Initialize the number of the examined tables
        table_in_page = -1
        pdf = pdfplumber.open(pdf_path)
        page_tables = pdf.pages[pagenum]
        tables = page_tables.find_tables()
        if len(tables) != 0:
            table_in_page = 0

        process_tables(tables, filepath, pagenum, text_from_tables)

        # Find all the elements
        page_elements = [(element.y1, element) for element in page._objs]
        # Sort all the element as they appear in the page
        page_elements.sort(key=lambda a: a[0], reverse=True)

        # Find the elements that composed a page
        for i, component in enumerate(page_elements):
            # Extract the element of the page layout
            element = component[1]

            # Check the elements for tables
            if table_in_page == -1:
                pass
            else:
                if is_element_inside_any_table(element, page, tables):
                    table_found = find_table_for_element(element, page,tables)
                    if table_found == table_in_page and table_found is not None:
                        page_content.append(text_from_tables[table_in_page])
                        page_text.append('table')
                        line_format.append('table')
                        table_in_page += 1

                    # Pass this iteration because the content of this element was extracted from the tables
                    continue

            if not is_element_inside_any_table(element, page, tables):
                line_format, page_content = process_text_element(element, page_text, line_format, page_content)

        # Add the list of list as value of the page key
        text_per_page[f'Page_{pagenum}'] = [page_text, line_format, text_from_images, text_from_tables, page_content]

    # Close the pdf file object
    pdf.close()

    # For now just write to file.
    result = ''.join(text_per_page['Page_0'][4])
    with open("/path/to/processed-resume.pdf.txt", "w") as text_file:
        text_file.write(result)


# TODO: this needs a lot of refinement.
if __name__ == "__main__":
    pdf_path = '/path/to/any/test-resume.pdf'
    main(pdf_path)
\$\endgroup\$

1 Answer 1

9
\$\begingroup\$

Self-analysis

The analysis of your own code is pretty accurate, which is a great start

Style

This code follows PEP8 guidelines for the most part, which is also great

Docstrings

The docstrings are helpful, but some of them are missing. I would argue that specifying the types in the docstrings is not a good idea, as they are usually not checked by static type checkers (like mypy). There is no need to duplicate this information, as it is easy to miss when updating a function signature. If a documentation must be generated, modern tools use the type annotations and display them anyway.

In table_converter, the Example usage: section should be called Example:, as specified in the Google docstring styleguide. A real doctest should also be implemented: this is a free unit test, and the documentation cannot drift from the implementation anymore:

"""
Example:
    >>> table = [['Name', 'Age'], ['Alice', '23'], ['Bob', None]]
    >>> print(table_converter(table))
    |Name|Age|
    |Alice|23|
    |Bob|None|
"""

Typing

Using type annotations is great, but it's not more informative than comments or docstrings if not checked with a static type checker like mypy. For example, the table parameter of table_converter should be annotated with list[list[str | None]]. mypy also detects that in process_tables, table can be None, and therefore should not be injected into table_converter.

Also, a lot of them are missing or not precise enough (Any). It is particularly helpful for the gentle reader (and for mypy) to annotate empty collections ([], {}, set()) or optional variables (my_var: int | None = None) at their instantiation.

Duplication

The functions is_element_inside_any_table and find_table_for_element are duplicated: the use-case of is_element_inside_any_table can be achieved with find_table_for_element. If a boolean function is really needed, a simple wrapper around find_table_for_element would be enough.

Functions design

Some function interfaces are hard to follow.

text_extraction should not return line_text, as it complexifies its signature, and the caller can easily and independently call element.get_text() if needed.

find_table_for_element takes a whole LTPage as an input, but only uses its height, which is a violation of the principle of least knowledge. This makes this function harder to test. Also, this function does two thing: converting from one system of coordinates to the other, and then finding the table. It could probably be split in two parts.

process_tables has a poorly informative name, and mainly takes low-level arguments, except for pdf_path. text_from_tables could simply be initialized as an empty list inside the function and then returned instead of being passed as an input argument. Also, there is no need to provide whole Tables, the length of the list is enough.

In process_text_element, page_text is modified during the execution of the function like line_format and page_content, but is not returned. This function should either not return anything, or return the data that it actually produced. The fact that three arguments out of the four are mutated show that a data structure is probably missing.

Variable in global scope

The pdf_path variable polutes the global scope, and there are conflicts with some of your pdf_path local variables (not a big deal here, but still a smell). It should be integrated to the main function, that should not take any arguments. A high level function that take paths (input and output) could be extracted from the main function if needed.

Managing IO

The input PDF is opened multiple times, and sometimes at the heart of the logic of the script, which sometimes make the code hard to follow. Ideally, I/O should be managed at the start/end of the script to fill/read from a properly defined data structure. When it is not possible to do that (memory constraints for example), I/O should be encapsulated (Using a class or functools.partial / Callable) and injected gracefully in the logic.

Also, in the main function the pdf = open(filepath, 'rb') statement does not achieve anything, as pdf is never used before being overwritten by pdf = pdfplumber.open(pdf_path). File openings should be handled with context managers when possible (with keyword).

When opening a file in write text mode, an encoding should be specified:

with open("/path/to/processed-resume.pdf.txt", "w", encoding="utf-8") as text_file:
    ...

Use of two libraries

Using two libraries to interact with PDFs instead of one can lead to problems that are visible here: multiple file openings, data structure conversions... Sticking to a single one can help a lot. pdfplumber depends on pdfminer.six, but it does not really help here. Have you considered only using pdfplumber to deal with the tables and the text? (See this issue)

Exceptions

In extract_tables, raising exceptions and catching them immediatly in the same function does not seem right. Catching Exception is usually not a good idea, and in this case, the None returned is not even handled in process_tables.

Lack of data structures

This line clearly shows that a data structure is lacking:

text_per_page[f'Page_{pagenum}'] = [page_text, line_format, text_from_images, text_from_tables, page_content]

The type of the variables in the list are heterogeneous, and the list has always the same size. Another clue is the presence of a magic number to access the data here:

result = ''.join(text_per_page['Page_0'][4])

Using Page_{pagenum} as keys makes things too complicated for an internal data structure. Why not using a simple list instead of a dictionary?

Overcomplicated sorting

This part can be simplified:

page_elements = [(element.y1, element) for element in page._objs]
# Sort all the element as they appear in the page
page_elements.sort(key=lambda a: a[0], reverse=True)

# Find the elements that composed a page
for i, component in enumerate(page_elements):
    # Extract the element of the page layout
    element = component[1]
  • i and enumerate are not needed
  • _objs is a private member of page and should not be accessed. Iterating over page actually yields the same thing, and is less likely to break in a next release of pdfminer
  • Storing a list of tuples is not needed
page_elements = sorted(page.objects, key=lambda elem: elem.y1, reverse=True)
for element in page_elements:
    ...

Comments

Some of the comments are not that informative:

# Extract the information of the table
table = extract_table(pdf_path, pagenum, table_num)
# Convert the table information in structured string format
table_string = table_converter(table)
# Append the table string into a list
text_from_tables.append(table_string)

Focusing on proper naming and type annotations is probably more helpful.

Parallelization

Before introducing parallelization, a profiling should be performed (with cProfile for example) to see where the main bottleneck is.

Refactor

Here is an attempt of refactor. mypy 1.8.0 passes in strict mode on python 3.11 (Windows). A simple PDF file available here has been used to verify that the behavior has not changed too much.

from typing import Optional, Iterable, NamedTuple, Sequence, Iterator

import pdfplumber
from pdfminer.high_level import extract_pages
from pdfminer.layout import LTPage, LTTextContainer, LTChar, LTComponent
from pdfminer.utils import Rect
from pdfplumber.page import Page
from pdfplumber.table import Table


class TextElement(NamedTuple):
    """A data structure to store elements extracted from a PDF file,
    formatted as text
    """

    text: str
    text_format: list[str] | str
    content: str


class LTTextTable(LTComponent):
    """A container to hold the text of a PDF table"""

    def __init__(self, text: str, bbox: Rect) -> None:
        super().__init__(bbox)
        self.text = text

    def __contains__(self, item: object) -> bool:
        """Returns ``True`` if the bounding box of the provided
        item is entirely contained in this instance
        """
        if isinstance(item, LTComponent):
            return (
                self.x0 <= item.x0
                and self.y0 <= item.y0
                and self.x1 > item.x1
                and self.y1 > item.y1
            )
        raise NotImplementedError


def extract_text_formats(element: LTTextContainer[LTComponent]) -> list[str]:
    """
    Extracts the unique formats (font names and sizes) from a given element.

    Parameters:
        element: The element from which the formats are extracted.

    Returns:
        A list of unique formats.
    """
    line_formats = set()
    for text_line in element:
        if isinstance(text_line, LTTextContainer):
            for character in text_line:
                if isinstance(character, LTChar):
                    format_info = f"{character.fontname}, {character.size}"
                    line_formats.add(format_info)
    return list(line_formats)


def format_table(table: list[list[str | None]]) -> str:
    """
    Converts a 2D table into a string format, where each cell is separated by '|'
    and each row is on a new line. Newline characters in cells are replaced with spaces,
    and None values are converted to the string 'None'.

    Parameters:
        table: The 2D table to convert.

    Returns:
        The string representation of the table.

    Example:
        >>> t = [['Name', 'Age'], ['Alice', '23'], ['Bob', None]]
        >>> print(format_table(t))
        |Name|Age|
        |Alice|23|
        |Bob|None|
    """
    converted_rows = []
    for row in table:
        cleaned_row = [
            item.replace("\n", " ") if item is not None else "None" for item in row
        ]
        converted_rows.append("|" + "|".join(cleaned_row) + "|")

    return "\n".join(converted_rows)


def convert_to_text_table(table: Table) -> LTTextTable:
    """Convert a ``pdfplumber`` Table to a ``pdfminer`` format

    Parameters:
        table: the table to convert

    Returns:
        an object that contains the text of the table and its bounding box
    """
    x0, y0_down, x1, y1_down = table.bbox
    page_y_max = table.page.bbox[3]
    return LTTextTable(
        text=format_table(table.extract()),
        bbox=(x0, page_y_max - y1_down, x1, page_y_max - y0_down),
    )


def find_table_for_element(
    bbox: LTComponent,
    bboxes_to_test: Iterable[LTTextTable],
) -> Optional[int]:
    """
    Finds the index of the table that a given element is inside on a PDF page.

    Parameters:
        bbox: The bounding box of the element to consider
        bboxes_to_test: An iterable of bounding boxes to test.

    Returns:
        The index of the bounding box that contains the element, or None if not found.
    """
    for i, bbox_to_test in enumerate(bboxes_to_test):
        if bbox in bbox_to_test:
            return i
    return None


def extract_text_from_page(
    lt_page: LTPage, text_tables: Sequence[LTTextTable]
) -> Iterator[TextElement]:
    """Iterate through a PDF page to extract the text and the tables

    Parameters:
        lt_page: The page from which the text should be extracted
        text_tables: The tables that are contained in this page

    Returns:
        An iterator that yields the sorted text elements contained in this page
    """
    page_elements = sorted(lt_page, key=lambda elem: elem.y1, reverse=True)
    table_in_page = 0
    for element in page_elements:
        table_index = find_table_for_element(element, text_tables)
        if table_index is None and isinstance(element, LTTextContainer):
            line_text = element.get_text()
            format_per_line = extract_text_formats(element)
            yield TextElement(text=line_text, text_format=format_per_line, content=line_text)
            continue
        if table_index == table_in_page:
            text_table = text_tables[table_index]
            yield TextElement(
                text="table",
                text_format="table",
                content=text_table.text,
            )
            table_in_page += 1


def iter_pdf_pages(
    pages: Iterable[Page], lt_pages: Iterable[LTPage]
) -> Iterator[list[TextElement]]:
    """
    Iterate through PDF pages from two iterators, and convert them to text. The iterator is
    exhausted as soon as one of the input is exhausted.

    Parameters:
        pages: An iterable of pdfplumber ``Page`` instances.
        lt_pages: An iterable of pdfminer ``LTPage`` instances.

    Returns:
        An iterator that yields a list of text elements for each page
    """
    for page, lt_page in zip(pages, lt_pages):
        tables: list[Table] = page.find_tables()
        text_tables = [convert_to_text_table(table) for table in tables]
        yield list(extract_text_from_page(lt_page, text_tables))


def convert_pdf(filepath: str, output_path: str) -> None:
    """
    Extract the text and the tables from a PDF file and the data to an output text file

    Parameters:
        filepath: Input path to a PDF
        output_path: Path to the .txt file to generate
    """
    pages = extract_pages(filepath)
    with pdfplumber.open(filepath) as pdf:
        text_per_page = list(iter_pdf_pages(pdf.pages, pages))

    result = "".join(element.content for element in text_per_page[0])
    with open(output_path, "w", encoding="utf-8") as text_file:
        text_file.write(result)


def main() -> None:
    filepath = r"/path/to/any/test-resume.pdf"
    output_path = r"/path/to/processed-resume.pdf.txt"
    convert_pdf(filepath, output_path)


if __name__ == "__main__":
    main()

\$\endgroup\$
2
  • \$\begingroup\$ Thanks, this answer is thorough. I particular like the consolidation of the structure and packages used. As well as your LTTextTable impl. \$\endgroup\$
    – Bob
    Commented Jan 6 at 0:11
  • \$\begingroup\$ Sufficient time has passed for others to have a go so marking this as the answer, Thanks again @rdesparbes. \$\endgroup\$
    – Bob
    Commented Jan 8 at 1:55

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.