MiraMonRaster driver (reading part) #12841
Open
+14,965
−855
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AbelPau
force-pushed
the
MiraMonRasterDriver(ReadPart)
branch
from
466f60a to
845dc38
Compare
AbelPau
force-pushed
the
MiraMonRasterDriver(ReadPart)
branch
from
845dc38 to
a5bf252
Compare
rouault
reviewed
Aug 1, 2025
| MiraMon Raster | ||
| ============== | ||
|
|
||
| .. shortname:: MiraMonRaster |
There was a problem hiding this comment.
Suggested change
| .. shortname:: MiraMonRaster | |
| .. versionadded:: 3.12 | |
| .. shortname:: MiraMonRaster |
| /************************************************************************/ | ||
| /* MMRBand() */ | ||
| /************************************************************************/ | ||
| MMRBand::MMRBand(MMRRel &fRel, CPLString osBandSectionIn) |
There was a problem hiding this comment.
Suggested change
| MMRBand::MMRBand(MMRRel &fRel, CPLString osBandSectionIn) | |
| MMRBand::MMRBand(MMRRel &fRel, const CPLString &osBandSectionIn) |
| nHeight = 0; | ||
| CPLError(CE_Failure, CPLE_AssertionFailed, | ||
| "The REL file '%s' contains a documented \ | ||
| band with no explicit name. Section [%s] or [%s:%s].\n", |
There was a problem hiding this comment.
Suggested change
| band with no explicit name. Section [%s] or [%s:%s].\n", | |
| band with no explicit name. Section [%s] or [%s:%s].", |
| { | ||
| osBandName = CPLGetBasenameSafe(osRawBandFileName); | ||
| CPLString osAux = | ||
| CPLGetPathSafe(static_cast<const char *>(pfRel->GetRELNameChar())); |
There was a problem hiding this comment.
Suggested change
| CPLGetPathSafe(static_cast<const char *>(pfRel->GetRELNameChar())); | |
| CPLGetPathSafe(pfRel->GetRELNameChar()); |
| nHeight = 0; | ||
| CPLError(CE_Failure, CPLE_AssertionFailed, | ||
| "The REL file '%s' contains a documented \ | ||
| band with no explicit name. Section [%s] or [%s:%s].\n", |
There was a problem hiding this comment.
Suggested change
| band with no explicit name. Section [%s] or [%s:%s].\n", | |
| band with no explicit name. Section [%s] or [%s:%s].", |
Comment on lines
+190
to
+191
| int nBands = 0; | ||
| MMRBand **papoBand = nullptr; |
There was a problem hiding this comment.
using a std::vector<std::unique_ptr<MMRBand>> could avoid manual memory management
| // Preserving metadata | ||
|
|
||
| // Domain | ||
| const char *kMetadataDomain = "MIRAMON"; |
There was a problem hiding this comment.
Suggested change
| const char *kMetadataDomain = "MIRAMON"; | |
| static constexpr const char *kMetadataDomain = "MIRAMON"; |
|
|
||
| // Used to join Section and Key in a single | ||
| // name for SetMetadataItem(Name, Value) | ||
| const char *SecKeySeparator = "[$$$]"; |
There was a problem hiding this comment.
Suggested change
| const char *SecKeySeparator = "[$$$]"; | |
| static constexpr const char *SecKeySeparator = "[$$$]"; |
| @@ -482,6 +482,13 @@ if ((GDAL_BUILD_OPTIONAL_DRIVERS AND NOT DEFINED GDAL_ENABLE_DRIVER_ADRG AND NOT | |||
| add_subdirectory(frmts/iso8211) | |||
| endif() | |||
|
|
|||
| # Build frmts/miramon_common conditionally to drivers requiring it | |||
| if ((GDAL_BUILD_OPTIONAL_DRIVERS AND NOT DEFINED GDAL_ENABLE_DRIVER_MIRAMONVECTOR AND NOT DEFINED GDAL_ENABLE_DRIVER_MIRAMONRASTER) OR | |||
There was a problem hiding this comment.
there's likely something wrong in this condition, as the variable for the vector driver is OGR_ENABLE_DRIVER_MIRAMONRASTER . You should likely test OGR_BUILD_OPTIONAL_DRIVERS too. See above test for frmts/iso8211
Comment on lines
+21
to
+27
| #ifdef MSVC | ||
| #include "..\..\..\frmts\miramon_common\mm_gdal_functions.h" | ||
| #include "..\..\..\frmts\miramon_common\mm_gdal_constants.h" | ||
| #else | ||
| #include "../../../frmts/miramon_common/mm_gdal_functions.h" | ||
| #include "../../../frmts/miramon_common/mm_gdal_constants.h" | ||
| #endif |
There was a problem hiding this comment.
I believe MSVC test is not needed:
Suggested change
| #ifdef MSVC | |
| #include "..\..\..\frmts\miramon_common\mm_gdal_functions.h" | |
| #include "..\..\..\frmts\miramon_common\mm_gdal_constants.h" | |
| #else | |
| #include "../../../frmts/miramon_common/mm_gdal_functions.h" | |
| #include "../../../frmts/miramon_common/mm_gdal_constants.h" | |
| #endif | |
| #include "../../../frmts/miramon_common/mm_gdal_functions.h" | |
| #include "../../../frmts/miramon_common/mm_gdal_constants.h" |
|
|
rouault
reviewed
Aug 1, 2025
| import pytest | ||
|
|
||
| from osgeo import gdal | ||
|
|
There was a problem hiding this comment.
you can remove all @pytest.mark.require_driver("MiraMonRaster") if you just add
Suggested change
| pytestmark = pytest.mark.require_driver("MiraMonRaster") | |
rouault
reviewed
Aug 1, 2025
Comment on lines
+23
to
+24
|
|
||
| gdal_target_link_libraries(gdal_MiraMon PRIVATE PROJ::proj) |
There was a problem hiding this comment.
should not be needed:
Suggested change
| gdal_target_link_libraries(gdal_MiraMon PRIVATE PROJ::proj) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Implements reading part for the MiraMonRaster driver.
I hope the code is clear. I’ve reused some logic from the vector side, particularly for reading DBF files and certain metadata handling.
AI assistance was used only for consultation purposes — mainly for generating small Python snippets, which I subsequently reviewed and fully understood, as well as for improving the clarity of the English writing.
New code is in C++.
If any part is unclear or needs adjustment, I’ll be available in September to answer questions or consider any suggestions reviewers may have.
Tests covers a 98.7% of functions (missing test for GetColorInterpretation() and AssignRGBColorDirectly()) and a 82.2% of the lines. I have made an effort in "error cases" but sometimes you would need very bad data to get one.
What are related issues/pull requests?
#12293
Tasklist