Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing a string as a page number raises IndexError instead of TypeError. #2961

Closed
bwagner opened this issue Jan 2, 2024 · 8 comments
Closed

Comments

@bwagner
Copy link
Contributor

bwagner commented Jan 2, 2024

Description of the bug

import fitz
doc = fitz.open("some.pdf")
doc["0"]

raises IndexError:

IndexError: page 0 not in document

Which is misleading, since the page 0 happens to be in the document, it's just being addressed wrongly.

How to reproduce the bug

import fitz
doc = fitz.open("some.pdf")
doc["0"]

raises IndexError:

IndexError: page 0 not in document

Which is misleading, since the page 0 happens to be in the document, it's just being addressed wrongly.

PyMuPDF version

1.23.9rc1

Operating system

MacOS

Python version

3.12

bwagner added a commit to bwagner/PyMuPDF that referenced this issue Jan 2, 2024
@bwagner
Copy link
Contributor Author

bwagner commented Jan 2, 2024

I tried to fix it, but my changes don't come through when I install the fixed package. When patching the installed file directly, however, my changes are effective. If anyone could recommend what I need to do to have my fix to come through when installing, please let me know.

@julian-smith-artifex-com
Copy link
Collaborator

PyMuPDF in git changed a week or two ago to default to installing the rebased implementation as module fitz, not the classic implementation.

So you should probably be patching src/__init__.py, not src_classic/fitz_old.i.

[Apart from that, i'm wondering whether it might be better to simply do assert isinstance(i, int), because this feels more appropriate for what is a programming error, not a normal runtime error?]

bwagner added a commit to bwagner/PyMuPDF that referenced this issue Jan 3, 2024
@bwagner
Copy link
Contributor Author

bwagner commented Jan 3, 2024

Thank you!

@bwagner
Copy link
Contributor Author

bwagner commented Jan 3, 2024

I signed the CLA. Is anything else required for this PR to be applied?

bwagner added a commit to bwagner/PyMuPDF that referenced this issue Jan 3, 2024
@JorjMcKie
Copy link
Collaborator

I haven't looked at the proposed code yet. But as a reminder:
Integers are not the only allowed item types for loading a page!
Always valid is also the form page = doc[(chapter, pno)] with a tuple of two integers.
Therefore checking for integer only is incorrect.

@julian-smith-artifex-com
Copy link
Collaborator

[Reopening until the fix is released.]

@julian-smith-artifex-com
Copy link
Collaborator

This is fixed in release candidate 1.23.9rc2; see: #2996

@julian-smith-artifex-com
Copy link
Collaborator

Fixed in 1.23.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants