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

[doc] Mention exceptions that can be raised by numpy.load upon corrup… #28265

Closed

Conversation

serge-sans-paille
Copy link
Contributor

…ted files

While fuzzing numpy.load through google/oss-fuzz#13001, I realized that some exceptions could be raised upon malformed .npy files but were not documented.

This patch documents those exceptions.

…ted files

While fuzzing numpy.load through google/oss-fuzz#13001,
I realized that some exceptions could be raised upon malformed .npy files but
were not documented.

This patch documents those exceptions.
@mattip
Copy link
Member

mattip commented Feb 3, 2025

The style guilde mentions that

This section should be used judiciously, i.e., only for errors that are non-obvious or have a large chance of getting raised.

Typically would a user expect all possible values to be specified?

@serge-sans-paille
Copy link
Contributor Author

serge-sans-paille commented Feb 3, 2025 via email

@seberg
Copy link
Member

seberg commented Feb 3, 2025

Thanks for improving loadtxt fuzzing there, that is neat.

If others disagree, happy to change my opinion, but I would also not document these: I tend to think most "bad input" style errors don't need documentation.

One argument: Documenting (to me) mostly makes sense if someone might want to write a try/except for them (or if correct input can raise for not extremely obscure reasons). But if that was the case here, we probably would have to wrap up those errors into a single IOError or CorruptedFile error to enable a try/except.

@serge-sans-paille
Copy link
Contributor Author

@seberg I'd happily follow your advice there. It was looking borderline to me, but I thought I'd mention it and discuss the situation around a small patch.

@ngoldbaum
Copy link
Member

I think I agree with Matti and Sebastian not to document these, since if these errors are triggered users should see these errors and it would be bad to mask them with an overzealous try/except clause.

@ngoldbaum ngoldbaum closed this Feb 4, 2025
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.

4 participants