-
Notifications
You must be signed in to change notification settings - Fork 19
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
Update with local installation info #484
Conversation
python/README.md
Outdated
To install versions of the package under active development, clone the repo and open the `python` directory: | ||
|
||
``` | ||
gh repo clone geoarrow/geoarrow-rs |
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.
I'd prefer standard git clone
syntax
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.
Done.
python/README.md
Outdated
|
||
``` | ||
gh repo clone geoarrow/geoarrow-rs | ||
cd geoarrow-rs/python |
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.
You need to additionally cd
into one of the python packages. You can't build anything from the python dir.
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.
👍
python/README.md
Outdated
pip install . | ||
``` | ||
|
||
You may need to install dependencies such as `maturin` and `pyarrow`. |
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.
I wouldn't mention pyarrow here, (or at least in a different section) as that's an optional runtime dependency, not a build dependency. Maturin is a build dependency but not a runtime dependency.
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.
Yeah, updated in latest version.
python/README.md
Outdated
From there you can install the package locally with `pip`: | ||
|
||
``` | ||
pip install . | ||
``` |
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.
For local development I'd suggest using maturin directly instead.
cd python/core
virtualenv env
source ./env/bin/activate
pip install -U maturin
maturin develop
That will build the wheel and install into the current virtualenv. You should note that's a debug build, so it'll run slower though. Use --release
to build a release build.
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.
Tested and now added.
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.
I may move this file to a different location later, as I had in mind that python/docs/DEVELOP.md
is instructions to build the docs, but this is fine for now.
Yeah, I noticed that these were the doc-building docs, hence trying to put them in the README. Optimal place: |
Possibly. It'll get more confusing/complex in the future when we also have |
No description provided.