search

ERDDAP / erddap

ERDDAP is a scientific data server that gives users a simple, consistent way to download subsets of gridded and tabular scientific datasets in common file formats and make graphs and maps. ERDDAP is a Free and Open Source (Apache and Apache-like) Java Servlet from NOAA NMFS SWFSC Environmental Research Division (ERD).
Creative Commons Zero v1.0 Universal
76 stars 54 forks source link

"EDDTableAggregateRows" - Time, Latitude and Longitude min/max values are calculated from the first child only #145

Closed MarcoAlbaETT closed 1 month ago

MarcoAlbaETT commented 2 months ago

Describe the bug EDDTableAggregateRows range for time is not the min/max of all the child. Also other variables, like latitude and longitude have this problem. Some variables are not affected by this problem and the min/max value is calculated from all the child

To Reproduce

  1. Create a "EDDTableAggregateRows" datasets with two (or more) child of type ""EDDTableFromErddap"
  2. Time of the first datasets must be different from the other. As an example, first child has time from 2013-01-01 to 2020-03-31 and the second one has data from 2024-01-01 to today
  3. The "EDDTableAggregateRows" time will be from 2013-01-01 to 2020-03-31

With real data, if you create this dataset:

<dataset type="EDDTableAggregateRows" datasetID="TS_SLEV_TAD" active="true">
    <reloadEveryNMinutes>60</reloadEveryNMinutes>
    <dataset type="EDDTableFromErddap" datasetID="TS_SLEV_TAD_15_composite" active="true" >
        <reloadEveryNMinutes>60</reloadEveryNMinutes>
        <sourceUrl>https://er1webapps.emodnet-physics.eu/erddap/tabledap/TS_SLEV_TAD_15</sourceUrl>
    </dataset>
    <dataset type="EDDTableFromErddap" datasetID="TS_SLEV_TAD_800_composite" active="true" >
        <reloadEveryNMinutes>60</reloadEveryNMinutes>
        <sourceUrl>https://er1webapps.emodnet-physics.eu/erddap/tabledap/TS_SLEV_TAD_800</sourceUrl>
    </dataset>
</dataset>

https://er1webapps.emodnet-physics.eu/erddap/tabledap/TS_SLEV_TAD_15 time is 2013-06-07T09:00:00Z to 2013-06-13T09:57:15Z https://er1webapps.emodnet-physics.eu/erddap/tabledap/TS_SLEV_TAD_800 time is 2022-09-01T00:03:00Z to 2023-12-19T11:03:00Z The EDDTableAggregateRows TS_SLEV_TAD time is 2013-06-07T09:00:00Z to 2013-06-13T09:57:15Z (instead of 2013-06-07T09:00:00Z to 2023-12-19T11:03:00Z) Same problem with latitude and longitude. Other variable (like SLEV) min/max are calculated in the right way.

Expected behavior time range, latitude and longitude of the EDDTableAggregateRows dataset to be the min / max of all the child datasets

Screenshots not applicable

Server I'm using ERDDAP Version 2.23 in a docker container using axiom/docker-erddap:2.23-jdk17-openjdk image

Desktop (please complete the following information):

Additional context None

MarcoAlbaETT commented 2 months ago

Hi! I've (probably) found the problem in the code for the time variable in the EDVTimeStamp class.
I'll make some test and if all is fine I'll make a pull request with the changes.

I'm still working on the same problem for latitude, longitude and depth variables.

ChrisJohnNOAA commented 2 months ago

@MarcoAlbaETT That's great. I've assigned this issue to you since you're working on it. Let me know if you have any questions.

MarcoAlbaETT commented 2 months ago

Hi @ChrisJohnNOAA, the bug fix is ready. Sadly I've trouble running the tests. I've changed only EDDTableAggregateRows.java class, but the tests fail for EDVTimeStampTests. I think this is something related to the language settings of my pc. Is it fine to make a pull request without a succesful test?

ChrisJohnNOAA commented 2 months ago

Yeah, go ahead and make the pull request. I'm aware that some tests can fail on other machines and have been trying to find and fix those to make them more reliable.