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

(How) is the rasterio.merge function taking into account different crs for the rasters #2696

Closed
VehpuS opened this issue Dec 30, 2022 Discussed in #2682 · 3 comments
Closed

Comments

@VehpuS
Copy link

VehpuS commented Dec 30, 2022

Discussed in #2682

Originally posted by VehpuS December 15, 2022
I have been looking into the implementation of rasterio.merge.merge to "prefetch" common bounds from a group of rasters. Once working on my implementation (since the function isn't currently too modular) I realized that I was getting errors when calculating common bounds of tiffs.

After some investigation, it turns out this was because the CRS of each tif was different, resulting in bounds given in different units. Before implementing my own solution, I looked at the merge implementation again but found no consideration for this. In the end, I implemented a solution based on rasterio.warp.transform_bounds.

I'm writing here to check if I missed something. If not, I would be happy to make changes to the merge function to either fix this or specify the assumption that all tiffs (and the target bounds parameters) are assumed to share the same CRS before running merge. I have a good approach for a solution if necessary, but I wanted to confirm my suspicions before implementing a PR.

@snowman2
Copy link
Member

snowman2 commented Dec 30, 2022

You may be interested in #2573 if your main goal is to get the bounds and you have rasters of different CRS.

UPDATE: After further inspection, that is not the case. They must have the same CRS.

@sgillies
Copy link
Member

@VehpuS the CRS requirement is written at https://github.com/rasterio/rasterio/blob/main/rasterio/merge.py#L108-L109 and is unlikely to change. Rasterio's merge is about merging rasters that are on the same grid. Different grids or CRS would require warping and that's a different process with different code and performance.

@VehpuS
Copy link
Author

VehpuS commented Dec 31, 2022

@VehpuS the CRS requirement is written at https://github.com/rasterio/rasterio/blob/main/rasterio/merge.py#L108-L109 and is unlikely to change. Rasterio's merge is about merging rasters that are on the same grid. Different grids or CRS would require warping and that's a different process with different code and performance.

I must have missed this part of the docstring when copying the implementation over to extract the common bounds logic (the original reason I looked at the code by itself during my worktime). Glad it's there, though, and I definitely feel stupid for missing it 😅!

I found value in being able to perform local merges with different projections, as sometimes happens when composing data from various sources for satellite image processing. But it may not be relevant for the library itself, which I acknowledge. If that's the case, feel free to close, but if not, I'll hapilly continue improving the pull request.

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

3 participants