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

swig/python/README.rst: Expand building from source section #9993

Merged
merged 2 commits into from
May 29, 2024

Conversation

gdt
Copy link
Contributor

@gdt gdt commented May 23, 2024

Expand (and reorg a bit) the guidance for building and installation, separating building from source and using packaging systems.

Mostly the only new content is

  • explaining how to run cmake and run swig, when building from source against an already-installed GDAL (needed for some packaging systems)

  • explain that PyPi hosts a version with swig already run and that one can use that to manually build a wheel (useful for some packaging systems, and to others to understand what's going on)

Expand (and reorg a bit) the guidance for building and installation,
separating building from source and using packaging systems.

Mostly the only new content is

  - explaining how to run cmake and run swig, when building from
    source against an already-installed GDAL (needed for some
    packaging systems)

  - explain that PyPi hosts a version with swig already run and that
    one can use that to manually build a wheel (useful for some
    packaging systems, and to others to understand what's going on)

Signed-off-by: Greg Troxel <gdt@lexort.com>
@rouault
Copy link
Member

rouault commented May 23, 2024

FYI, swig/python/README.rst is what is used to put in the "sdist" package and is displayed on https://pypi.org/project/GDAL/

Perhaps part of the information of this PR should be put also (or exclusively?) in https://gdal.org/api/python_bindings.html (doc/source/api/python_bindings.rst) ?

@gdt
Copy link
Contributor Author

gdt commented May 23, 2024

I have a bias that for open source code, the primary path for users is to build it from source, even if many of them do not. And a bias that the information on how to build belongs with the sources and with the distributed tarballs.

Whether it is in README, which is perhaps co-opted by github/pypi disease which has redefined tradtional rules, is not important. I usually split it to INSTALLING.md or some such, and that would be how to build via source and get it via packaging, with the how to use it and what the big picture is in README.

Overall, I don't think a bit more info in README showing up is a problem. It's really not that much bigger, and the tutorial about conda is pretty large anyway (and seems pretty tutorial, vs trying to be as brief as possible assuming the reader generally knows how to build things, but doesn't know what this package needs). I had no idea there was a pre-swigged tarball (it's not a github CI artifact on the release) and I suspect other people struggling with packaging might not either.

@gdt
Copy link
Contributor Author

gdt commented May 23, 2024

I could also see putting the "how to use" up front, but I was trying to rototill as little as possible while addressing the 'build from source' as a first-class option, and to make sure any other packager would be quickly led to really understand.

@rouault
Copy link
Member

rouault commented May 23, 2024

Overall, I don't think a bit more info in README showing up is a problem.

There might be a misunderstanding. I'm not against additions. My above comment was just pointing out that there are 2 similar pages in our documentation: one that is embedded in the artifact generated by "python setup.py sdist" (the page you edited), and the one that is displayed on gdal.org (whose source is in doc/source/api/python_bindings.rst) . . They have similar but not exactly identical content, because the audience isn't necessarily the same. I believe that the part about "Building just the bindings, from the full tree" typically would be best displayed on gdal.org

@coveralls
Copy link
Collaborator

coveralls commented May 23, 2024

Coverage Status

coverage: 69.131% (-0.002%) from 69.133%
when pulling 046f747 on gdt:master
into 12dab86 on OSGeo:master.

@gdt
Copy link
Contributor Author

gdt commented May 24, 2024

I see. Well, I think the instructions for dealing with the sources belong in the sources. I would never have looked for them in API docs, which I expect to be about using the code once installed, and not at all about installing it. And I would think if moved, we should move the conda info too?

@rouault
Copy link
Member

rouault commented May 24, 2024

And I would think if moved, we should move the conda info too?

I would keep the existing info in swig/python/README.rst (other people have already worked on those docs a few months ago, cf #9405). We "advertize" conda on purpose in swig/python/README.rst in particular because it is not that easy to get "pip install gdal" to work on non-Unix systems (Windows to be clear) that lack a convenient way of getting the GDAL headers and libraries. Windows users keep complaining about "pip install gdal" not working for them, so our answer is "well, just use 'conda install gdal instead'".

swig/python/README.rst Outdated Show resolved Hide resolved
@rouault rouault merged commit 1118039 into OSGeo:master May 29, 2024
35 checks passed
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.

3 participants