Skip to content

feat: add support for roman numeral page numbers in TOC - #481

Open
denis-samatov wants to merge 1 commit into
VectifyAI:mainfrom
denis-samatov:feat/support-roman-numerals
Open

feat: add support for roman numeral page numbers in TOC#481
denis-samatov wants to merge 1 commit into
VectifyAI:mainfrom
denis-samatov:feat/support-roman-numerals

Conversation

@denis-samatov

Copy link
Copy Markdown

Summary

  • Adds support for parsing Roman numeral page numbers (e.g. i, ii, iv, xii, IV) in Table of Contents structures.
  • Resolves Add support for roman numeral page numbers (ex. ii, iv)聽#164: "Add support for roman numeral page numbers (ex. ii, iv)".
  • Prevents documents whose introductory, preface, or TOC pages use Roman numerals from failing convert_page_to_int, being skipped by calculate_page_offset and add_page_offset_to_toc_json, and unnecessarily falling back to fallback search.

Changes

  1. pageindex/utils.py:
    • Added roman_to_int(s: str) -> int | None with standard Roman numeral validation (case-insensitive, supporting i through m).
    • Updated convert_page_to_int(data) to attempt integer conversion first, and if ValueError, parse valid Roman numerals into integers.
  2. tests/test_roman_numerals.py:
    • Added unit test suite covering valid Roman numerals (single digits, multi-digits, subtractive notation, mixed case, whitespace).
    • Added tests for invalid strings/types ensuring graceful handling without unexpected conversions.
    • Added integration test verifying convert_page_to_int on mixed Arabic and Roman numeral TOC structures.

Verification

  • Ran pytest tests/test_roman_numerals.py: 41 passed.
  • Ran full test suite: 471 passed, 54 skipped, 0 failures.

Closes #164

- Add roman_to_int helper to parse roman numeral page numbers (e.g. 'i', 'ii', 'iv', 'xii', 'IV')
- Update convert_page_to_int to convert roman numerals into integers
- Prevents roman-numeral preface and introductory sections from falling back to unindexed/None pages
- Add comprehensive test suite in tests/test_roman_numerals.py

Closes VectifyAI#164
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

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.

Add support for roman numeral page numbers (ex. ii, iv)

1 participant