Skip to content

Add support for EDDGridLonPM180 to fix longitudes greater than 360#379

Merged
ChrisJohnNOAA merged 5 commits intoERDDAP:mainfrom
SethChampagneNRL:feature/lonGT360
Oct 23, 2025
Merged

Add support for EDDGridLonPM180 to fix longitudes greater than 360#379
ChrisJohnNOAA merged 5 commits intoERDDAP:mainfrom
SethChampagneNRL:feature/lonGT360

Conversation

@SethChampagneNRL
Copy link
Contributor

@SethChampagneNRL SethChampagneNRL commented Oct 20, 2025

Description

I am adding support for ECMWF Grib files pulled form their Python API https://pypi.org/project/ecmwf-opendata/

These Grib files have longitude in the range 180 to 540. I would like to correct these into -180 to 180. The basic logic for this was already supported by EDDGridLonPM180, but I needed to increase the maximum value from 360 to 540. Adding an optional dataset parameter did the trick.

 <dataset type="EDDGridLonPM180" datasetID="ECMWF-FIXED" active="true">
    <maxSourceLon>540</maxSourceLon>
    <dataset type="EDDGridFromNcFiles" datasetID="ECMWF" active="true">
        <!-- redacted internals -->
    </dataset>
</dataset>

Another issue in the ECMWF dataset is the latitudes running from 90 to -90 instead of -90 to 90. I would like to also correct these, but I have not figured out a solution yet.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist before requesting a review

  • I have performed a self-review of my code
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@rmendels
Copy link
Collaborator

@SethChampagneNRL @ChrisJohnNOAA you said "Another issue in the ECMWF dataset is the latitudes running from 90 to -90 instead of -90 to 90. I would like to also correct these, but I have not figured out a solution yet." There are existing netcdf datasets that have latitudes running from 90 to -90 so there must be code already there to do what you want.

@SethChampagneNRL
Copy link
Contributor Author

@SethChampagneNRL @ChrisJohnNOAA you said "Another issue in the ECMWF dataset is the latitudes running from 90 to -90 instead of -90 to 90. I would like to also correct these, but I have not figured out a solution yet." There are existing netcdf datasets that have latitudes running from 90 to -90 so there must be code already there to do what you want.

I would not be surprised if this is already supported and I just am not aware of the feature.

@ChrisJohnNOAA
Copy link
Contributor

latitudes running from 90 to -90 instead of -90 to 90

From the documentation: https://erddap.github.io/docs/server-admin/datasets?_highlight=latitude#special-variables

For the "longitude" variable and the "latitude" variable:
Use the destinationNames "longitude" and "latitude" only if the units are degrees_east and degrees_north, respectively. If your data doesn't fit these requirements, use different variable names (for example, x, y, lonRadians, latRadians).
If you have longitude and latitude data expressed in different units and thus with different destinationNames, for example, lonRadians and latRadians, Make A Graph (datasetID.graph) will make graphs (for example, time series) instead of maps.

Are the datasets that currently do it using a destination variable named "latitude" or do they call it something else?

You might be able to use scale_factor to adjust the latitude.

<att name="scale_factor" type="float">-1</att>

@rmendels
Copy link
Collaborator

@SethChampagneNRL @ChrisJohnNOAA You can just use latitude and degrees_north. Here is one example:

https://coastwatch.pfeg.noaa.gov/erddap/griddap/erdVHsstaWS3day.html

In fact I think most if not all the VIIRS datasets go north to south, and there are others but I am too lazy to find them. I don't know what ERDDAP does internally but it doesn't seem to be bothered by it.

@SethChampagneNRL
Copy link
Contributor Author

The ECMWF dataset in our ERDDAP has the axis set as "latitude", and it gives a -0.25 spacing. It functions correctly in ERDDAP. ERDDAP retrieves the correct rows and writes them into netcdf. So swapping the direction of latitude isn't a showstopper, but it would be nice for standardization sake.

Using the scale_factor solution did change the latitude values, but causes the rows to get flipped upside down in the resulting netcdf.

@SethChampagneNRL SethChampagneNRL marked this pull request as ready for review October 21, 2025 15:22
Copy link
Contributor

@ChrisJohnNOAA ChrisJohnNOAA left a comment

Choose a reason for hiding this comment

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

The code itself looks good, but I'd really like to see a test for this. If there's a small data file it can be added to src/test/resources (https://github.com/ERDDAP/erddap/tree/main/src/test/resources).

If this needs large files for testing we can update the zip release file at: https://github.com/ERDDAP/erddapTest

@SethChampagneNRL
Copy link
Contributor Author

Added a unit test using an example ECMWF temperature file.
ERDDAP/erddapTest#1

@ChrisJohnNOAA
Copy link
Contributor

How big is the file? Can we add it to src/test/resources/[subfolder] or do we need to update the test resource zips?

@SethChampagneNRL
Copy link
Contributor Author

How big is the file? Can we add it to src/test/resources/[subfolder] or do we need to update the test resource zips?

It is ~7.7MB. I added it in a PR to erddapTest

@SethChampagneNRL
Copy link
Contributor Author

I think I can download a lower resolution grib and make it smaller. But it should still go to the zip file instead of erddap's repo imo.

@SethChampagneNRL
Copy link
Contributor Author

Never mind, ECMWF only offers 0.25 resolution.

@ChrisJohnNOAA
Copy link
Contributor

Never mind, ECMWF only offers 0.25 resolution.

I made a new release of the test data. The pom for this pull needs to be updated to use test1.05

@ChrisJohnNOAA
Copy link
Contributor

Never mind, ECMWF only offers 0.25 resolution.

I made a new release of the test data. The pom for this pull needs to be updated to use test1.05

@SethChampagneNRL and a pull request to update the JettyTest that lists the datasets.

@SethChampagneNRL
Copy link
Contributor Author

Never mind, ECMWF only offers 0.25 resolution.

I made a new release of the test data. The pom for this pull needs to be updated to use test1.05

@SethChampagneNRL and a pull request to update the JettyTest that lists the datasets.

Which test are you referring to?

@ChrisJohnNOAA
Copy link
Contributor

Never mind, ECMWF only offers 0.25 resolution.

I made a new release of the test data. The pom for this pull needs to be updated to use test1.05

@SethChampagneNRL and a pull request to update the JettyTest that lists the datasets.

Which test are you referring to?

JettyTests.testJsonld which failed in the GitHub test run.

@ChrisJohnNOAA ChrisJohnNOAA merged commit 859b5ea into ERDDAP:main Oct 23, 2025
3 checks passed
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