clarify error message on missing path

PyO3 / pyo3

Rust bindings for the Python interpreter

https://pyo3.rs

Other

11.44k stars 693 forks source link

clarify error message on missing path #4278

Closed H4vC closed 1 week ago

H4vC commented 1 week ago

fix for https://github.com/PyO3/pyo3/issues/4277

H4vC commented 1 week ago

This is a trivial change.

davidhewitt commented 1 week ago

Thanks for the PR! We actually have #4043 also improving this line, but that got stuck in review limbo. Would you be willing to help finish that one off instead please? You could pull those commits and add some extra ones on top.

The changes waiting on that PR were:

how to test it - just write a unit test that attempts to read from a nonexistent file at the bottom of the same file
some suggestions to actually include PYO3_CROSS_LIB_DIR in the error message

mrexodia commented 1 week ago

Thanks for the PR! We actually have #4043 also improving this line, but that got stuck in review limbo. Would you be willing to help finish that one off instead please? You could pull those commits and add some extra ones on top.

The changes waiting on that PR were:

how to test it - just write a unit test that attempts to read from a nonexistent file at the bottom of the same file

some suggestions to actually include PYO3_CROSS_LIB_DIR in the error message

This change as-is would have saved me a lot of time already. Considering it's a 1-line change wouldn't it be good to merge this as-is and then refactor later?

davidhewitt commented 1 week ago

I completely agree and am sorry that this wasn't fixed sooner. Nobody wants time wasted by bad error messages.

I found a moment to push to #4043 just now, so let's merge that one. Thank you both for reminding me that this error message matters. (You were not the first folks to hit this case.)

mrexodia commented 1 week ago

Awesome, thanks for the quick fix!