MHKiT-Software / MHKiT-Python

MHKiT-Python provides the marine renewable energy (MRE) community tools for data processing, visualization, quality control, resource assessment, and device performance.
https://mhkit-software.github.io/MHKiT/
BSD 3-Clause "New" or "Revised" License
47 stars 45 forks source link

Convert river and tidal modules to xarray #285

Closed akeeste closed 3 months ago

akeeste commented 5 months ago

This PR converts the river and tidal modules to be based in xarray

TODO list

Modules completed:

ssolson commented 4 months ago

@akeeste could you take a look at fixing the PR failed tests before I review?

_______________________ TestIO.test_load_usgs_data_daily _______________________

self = <mhkit.tests.river.test_io_usgs.TestIO testMethod=test_load_usgs_data_daily>

    def test_load_usgs_data_daily(self):
        file_name = join(datadir, "USGS_0831[30](https://github.com/MHKiT-Software/MHKiT-Python/actions/runs/8147648568/job/22268844047?pr=285#step:6:31)00_Jan2019_daily.json")
        data = river.io.usgs.read_usgs_file(file_name)

        expected_index = pd.date_range("2019-01-01", "2019-01-[31](https://github.com/MHKiT-Software/MHKiT-Python/actions/runs/8147648568/job/22268844047?pr=285#step:6:32)", freq="D")
        self.assertEqual(data.columns, ["Discharge, cubic feet per second"])
>       self.assertEqual((data.index == expected_index.tz_localize("UTC")).all(), True)
E       AssertionError: False != True
akeeste commented 4 months ago

@akeeste could you take a look at fixing the PR failed tests before I review?

_______________________ TestIO.test_load_usgs_data_daily _______________________

self = <mhkit.tests.river.test_io_usgs.TestIO testMethod=test_load_usgs_data_daily>

    def test_load_usgs_data_daily(self):
        file_name = join(datadir, "USGS_0831[30](https://github.com/MHKiT-Software/MHKiT-Python/actions/runs/8147648568/job/22268844047?pr=285#step:6:31)00_Jan2019_daily.json")
        data = river.io.usgs.read_usgs_file(file_name)

        expected_index = pd.date_range("2019-01-01", "2019-01-[31](https://github.com/MHKiT-Software/MHKiT-Python/actions/runs/8147648568/job/22268844047?pr=285#step:6:32)", freq="D")
        self.assertEqual(data.columns, ["Discharge, cubic feet per second"])
>       self.assertEqual((data.index == expected_index.tz_localize("UTC")).all(), True)
E       AssertionError: False != True

@ssolson yeah definitely, i'll see what's going on and ping you when it's ready

akeeste commented 3 months ago

@ssolson Sorry for the wait, I think I have the tests passing now. I'll double check tomorrow morning. I was having some issues converting river.io.usgs.py and especially variable_interpolation and turbulence_intensity in river.io.d3d.py to xarray. The calls to scipy.interp.griddata were proving difficult with d3d data in xr.Dataset format. For now, I reverted those functions to use pandas, but added the option to return xarray datasets. So river.io and tidal.io functions are not yet based in xarray, but can now return xarray.

akeeste commented 3 months ago

@ssolson okay tests are finally passing! I had to go back and fix a few changes I made to how some river.resource functions output their data, but that is all working as it was before. Again noting that the river and tidal io functions can now output xarray, but are not based in xarray yet because they were tricky

akeeste commented 3 months ago

Thanks for the review @ssolson . I addressed all of your review comments, ported my functions to your much better named utils.type_handling 😊, and can confirm that the river, tidal, and tidal performance examples all run without errors or warnings.

akeeste commented 3 months ago

Within test_utils, the name change from convert_to_dataarray to test_convert_to_dataarray made that test actually run. In convert_to_dataarray there was one minor inconsistency with how datasets are cast to dataarrays. the xr to_array function always makes the variable name a new dimension, which is not necessary in our scenario and makes the returned dataarray different from how all other types are converted. I fixed this case so the function works consistently