-
Notifications
You must be signed in to change notification settings - Fork 81
DOC: document how to use installed data files with editable installs #803
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
Conversation
3bc90fe to
2816ac6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dnicolodi. The added text about how to use data files LGTM (modulo the minor open comments) and is useful.
It doesn't quite address the "can't use the NumPy C API from another package when NumPy itself is installed in editable mode". It can be added to this PR, probably as a .. warning:: note at the end, or left for a separate follow-up.
469c4f5 to
1bac81f
Compare
1bac81f to
75b3119
Compare
I feel that the numPy issue is too specific to be mentioned in the general documentation for editable installs. I think that the documentation now states clearly the reason why the headers cannot be used. Anything more specific would be more discoverable in the NumPy documentation or in a new "Papercuts" section of the documentation (maybe started by renaming the current "Limitations" section). |
Fair enough. And I like the Papercuts name. I can have a look at it, also seeing if/where it needs a warning in the NumPy docs. |
|
I’d personally go that way yes. But don’t feel super strongly about it either
|
|
I'll keep the note about the issue with |
The NumPy docs at https://numpy.org/devdocs/building/index.html have this warning at the bottom of the section on editable installs, so that part should be covered:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, and the added test is also nice. In it goes.

Replaces #786.