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

adds docs for resample, MODIS projection #730

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

lazarusA
Copy link
Collaborator

@lazarusA lazarusA commented Sep 4, 2024

Ok. I'm adding this because the usual just happened. I needed to do this, and yes, good luck finding docs or examples 😄 . Well, here, a version. It might need a second pass.

@asinghvi17 it would be also nice to include also your use cases.

Copy link
Collaborator

@asinghvi17 asinghvi17 left a comment

Choose a reason for hiding this comment

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

I think we need a lot more intro about what exactly this CRS/projection thingy is for new users who might have just loaded their first raster into memory.

docs/src/resample_proj.md Outdated Show resolved Hide resolved
docs/src/resample_proj.md Outdated Show resolved Hide resolved
Comment on lines +31 to +32
# ? is this the right ProjString ?, do we need to shift lat, lon?
SINUSOIDAL_CRS = ProjString("+proj=sinu +lon_0=0 +x_0=0 +y_0=0 +a=6371007.181 +b=6371007.181 +units=m +no_defs")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Users will be wondering what the hell a ProjString is, we should explain that

Copy link
Collaborator

@asinghvi17 asinghvi17 Oct 11, 2024

Choose a reason for hiding this comment

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

also @lazarusA why are you defining the ellipse here? That seems like a step too far. How about:

Suggested change
# ? is this the right ProjString ?, do we need to shift lat, lon?
SINUSOIDAL_CRS = ProjString("+proj=sinu +lon_0=0 +x_0=0 +y_0=0 +a=6371007.181 +b=6371007.181 +units=m +no_defs")
SINUSOIDAL_CRS = ProjString("+proj=sinu +lon_0=0 +type=crs")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Those tiles come/are set with very specific numbers. I think I did some tests, and you not always get the same numbers. BTW, feel free to merge this into your PR, I will probably not finish this anytime soon.

ras_epsg = resample(ras_sin; size=(1440,720), crs=EPSG(4326), method="sum")
````

and let's apply `shiftlocus` such that we can harmonize coordinates, which might be needed when building bigger datasets:
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does this do, what does it mean to harmonize coordinates?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I usually need to match to other variables, which are in a specific Lon/lat range.

docs/src/resample_proj.md Outdated Show resolved Hide resolved
@@ -73,6 +74,7 @@ export default defineConfig({
{ text: 'Methods',
items: [
{ text: 'Overview', link: '/methods' },
{ text: 'resample', link: '/resample_proj'},
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be capitalized IMO, or we could call the page Reprojection (since that's what it does - resampling to me implies a simple down- or up-sampling without nonlinearity).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe both? Reprojection and resample.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{ text: 'resample', link: '/resample_proj'},
{ text: 'Reprojection and resampling', link: '/resample_proj'},

@rafaqz
Copy link
Owner

rafaqz commented Sep 4, 2024

resample does shiftlocus for you, it has to be Start for GDAL

@asinghvi17
Copy link
Collaborator

I'll write a dropdown about what CRS is that we can include here, new users can then click on it if they need to.

Copy link

codecov bot commented Sep 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.36%. Comparing base (570fd17) to head (c6e946f).
Report is 18 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #730      +/-   ##
==========================================
- Coverage   82.59%   82.36%   -0.23%     
==========================================
  Files          60       62       +2     
  Lines        4510     4537      +27     
==========================================
+ Hits         3725     3737      +12     
- Misses        785      800      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Plots = "91a5bcdd-55d7-5caf-9e0b-520d859cae80"
RasterDataSources = "3cb90ccd-e1b6-4867-9617-4276c8b2ca36"
Rasters = "a3a2b9e3-a471-40c9-b274-f788e487c689"
Shapefile = "8e980c4a-a4fe-5da2-b3a7-4b4b0353a2f4"
YAXArrays = "c21b50f5-aa40-41ea-b809-c0f5e47bfa5c"
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need YAX?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no.

@rafaqz
Copy link
Owner

rafaqz commented Sep 19, 2024

Is this ready to merge?

docs/Project.toml Outdated Show resolved Hide resolved
and let's apply `shiftlocus` such that we can harmonize coordinates, which might be needed when building bigger datasets:

````@example modis
locus_resampled = DimensionalData.shiftlocus(Center(), ras_epsg)
Copy link
Owner

Choose a reason for hiding this comment

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

What happens if we delete these? would be nice to avoid

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

mmm... yes, but then we don't show how to do it. Which is needed, or at least I needed it 😄 .

@lazarusA
Copy link
Collaborator Author

there some things that might help, like saying or referencing to what crs is or a ProjString. Other that, I'm not planning on adding more.

@asinghvi17 asinghvi17 mentioned this pull request Oct 11, 2024
5 tasks
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.

3 participants