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_exceptions.i: Add Context Manager for handling Python exception state #6637

Merged
merged 5 commits into from
Nov 10, 2022
Merged

swig/python_exceptions.i: Add Context Manager for handling Python exception state #6637

merged 5 commits into from
Nov 10, 2022

Conversation

neilflood
Copy link
Contributor

Allow the state of using exceptions to be set/reset locally, within a Python with statement

The Python gdal/ogr/osr/gnm modules each have a global state flag determining whether or not errors raise Exceptions. It defaults to not, and can be set or unset. However, because this is global, conflicts can arise in what a section of code requires, sometimes in code from other libraries. To avoid interfering with other code, one often needs to save current state, set desired state, perform some actions, then reset to saved state.

This PR adds a Python Context Manager class which handles this process, so that a Python with statement can have a desired state within that block, and not interact with the global state.

It is added to the generic python_exceptions.i SWIG code, so it appears in each of the different GDAL modules.

Copy link
Member

@rouault rouault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind also comitting the regnerated bindings file ?
Cf https://github.com/OSGeo/gdal/blob/master/CONTRIBUTING.md#build-swig to have SWIG 4.0.2 and also note you need to pass -DSWIG_REGENERATE_PYTHON=ON to CMake

It would be great to have testing of this class, for example in autotest/gcore/basic_test.py

@neilflood
Copy link
Contributor Author

Thanks @rouault
Yes, I have tested locally (Ubuntu) using -DSWIG_REGENERATE_PYTHON and swig-4.0.2, all seems fine.

Have added the four regenerated bindings, and a simple test where you suggested. Thank you!

@rouault rouault added this to the 3.7.0 milestone Nov 9, 2022
@rouault rouault merged commit 8633aa6 into OSGeo:master Nov 10, 2022
rouault added a commit to rouault/gdal that referenced this pull request Mar 17, 2023
PR OSGeo#6637 introduced a gdal/ogr/osr.ContextMgr() class, which I forgot
when committing 26151f7
("Python bindings: add gdal.enable_exceptions() context manager") in PR

So:
- remove this redundant gdal/ogr/osr.enable_exceptions()
- remove gdaltest/ogrtest.enable_exceptions()
- use ContextMgr() instead
- and make its useExceptions argument default to True
rouault added a commit to rouault/gdal that referenced this pull request Mar 17, 2023
PR OSGeo#6637 introduced a gdal/ogr/osr.ContextMgr() class, which I forgot
when committing 26151f7
("Python bindings: add gdal.enable_exceptions() context manager") in PR

So:
- remove this redundant gdal/ogr/osr.enable_exceptions()
- remove gdaltest/ogrtest.enable_exceptions()
- use ContextMgr() instead
- and make its useExceptions argument default to True
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.

2 participants