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
Add RFC 91 text: GDALDataset::Close() method #7108
Conversation
Thanks @rouault . I was one of those that used the ugly hacky error detection. |
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.
Looks great to me! Some minor suggestions that are just polish.
------------- | ||
|
||
The destructor of gdal.Dataset is modified to test the return value of GDALClose() | ||
and emits a CPLError(CE_Failure, ...) if the error state is clean (normally |
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.
is clean -> is not clean ??
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.
no, the current formulation is intended. See commit rouault@c3c37ea
This is to make sure that the gdal.UseExceptions() mode throws an exception. It needs CPLGetLastErrorType() == CE_Failure, so if GDALClose() != CE_None and CPLGetLastErrorType() == CE_None, we issue a CPLError(CE_Failure, ...)
It looks good to me. A minor addition to the example to push it to safer side MyDataset::~MyDataset()
{
try {
MyDataset::Close();
} catch (...) {
// swallow error, optionally log it but ensure the action never throws
}
} |
added |
Rendered view in https://github.com/rouault/gdal/blob/rfc91_text/doc/source/development/rfc/rfc91_dataset_close.rst