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

Python bindings ogr module removed from gdal 3.2 / Not in release Notes #3149

Closed
cpaulik opened this issue Nov 5, 2020 · 11 comments
Closed

Comments

@cpaulik
Copy link

cpaulik commented Nov 5, 2020

Expected behavior and actual behavior.

import ogr

gives

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'ogr'

in gdal 3.2.

I know that we should use from osgeo import ogr but since I could not find this breaking change in the release notes I was wondering if it was removed by mistake or forgotten in the release notes.

It still works under gdal 3.1

Operating system

Arch linux

GDAL version and provenance

gdal 3.2 from conda-forge

@cpaulik cpaulik changed the title Python bindings ogr removed from gdal 3.2 / Not in release Notes Python bindings ogr module removed from gdal 3.2 / Not in release Notes Nov 5, 2020
@rouault
Copy link
Member

rouault commented Nov 5, 2020

"import ogr" still works fine for me with 3.2 . I can't think of any related change.

@cpaulik
Copy link
Author

cpaulik commented Nov 5, 2020

Thanks. I've created conda-forge/gdal-feedstock#419 to see if it is a conda-forge issue.

Closing this for now.

@cpaulik cpaulik closed this as completed Nov 5, 2020
@cpaulik
Copy link
Author

cpaulik commented Nov 5, 2020

conda-forge claimed to have changed nothing. But they needed to remove their import ogr statements in their python tests.

Does gdal have a CI test for this import? If so then something in the middle goes wrong.

@cpaulik cpaulik reopened this Nov 5, 2020
@ocefpaf
Copy link

ocefpaf commented Nov 5, 2020

"import ogr" still works fine for me with 3.2 . I can't think of any related change.

It did not work in our builds. Should we activate a flag or something to enable it?

@rouault
Copy link
Member

rouault commented Nov 5, 2020

@cpaulik I can actually reproduce the issue now. The fact that it worked was due to testing in a dev environement where "ogr" is a subdirectory... So it doesn't appear that "import ogr" is tested in CI. That said this has been deprecated for 10 years or so.

@idanmiara I believe this change might be caused by your rework of setup.py when moving the utilities under osgeo/scripts . The gdal.py, ogr.py, etc. files that used to be generated under swig/python/build/lib.XXXXXX are no longer there

I'm a bit hesitating what to do now: should we restore the "import ogr" way or just update our NEWS to reflect the change...

@idanmiara
Copy link
Collaborator

@idanmiara I believe this change might be caused by your rework of setup.py when moving the utilities under osgeo/scripts . The gdal.py, ogr.py, etc. files that used to be generated under swig/python/build/lib.XXXXXX are no longer there

I'm a bit hesitating what to do now: should we restore the "import ogr" way or just update our NEWS to reflect the change...

I guess I could have been mistaken to think that the py_modules section in setup.py is deprecated.
I hope this fixes this issue:
#3150

@ocefpaf
Copy link

ocefpaf commented Nov 5, 2020

I'm a bit hesitating what to do now: should we restore the "import ogr" way or just update our NEWS to reflect the change...

I'm only one data point and, while restoring it would be "easy," the cat is out of the bag IMO. Document the change and move on.

PS: thanks for all the work on gdal!

@rouault
Copy link
Member

rouault commented Nov 5, 2020

Polling the wider community : https://lists.osgeo.org/pipermail/gdal-dev/2020-November/052921.html

@cpaulik
Copy link
Author

cpaulik commented Nov 5, 2020

I'm happy with documenting it and moving forward. We've already changed our imports.

@jomey
Copy link
Contributor

jomey commented Nov 5, 2020

How about doing a quick poll here by reacting with 👍 for moving on and 👎 for restoring?

@hobu
Copy link
Contributor

hobu commented Nov 9, 2020

Let's leave it gone. We should have done this at 2.0 but we held off. It will be annoying for a few downstream Python API users, but it is a simple import statement fix.

@rouault rouault closed this as completed in 843f5cb Nov 9, 2020
rouault added a commit that referenced this issue Nov 9, 2020
NEWS and MIGRATION_GUIDE.TXT: document removal of deprecated 'import gdal' way of Python bindings. Fixes #3149 [ci skip]
rouault added a commit that referenced this issue Nov 9, 2020
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

No branches or pull requests

6 participants