search

hainegroup / oceanspy

A Python package to facilitate ocean model data analysis and visualization.
https://oceanspy.readthedocs.io
MIT License
101 stars 32 forks source link

remove pin to intake #419

Closed Mikejmnez closed 7 months ago

Mikejmnez commented 7 months ago

I expect some test to fail but overall should close #415

Reason I expect test to fail?:

The Data subfolder that is used for some testings is downloaded directly from zenodo: The following snippet I copied directly from CI testing . Downloading data from 'https://zenodo.org/record/5832607/files/Data.tar.gz?download=1' to file '/home/runner/.cache/pooch/8cba001ed979f3b28c5ae4e283495e67-Data.tar.gz'.

During CI testing, Data.tar is downloaded and untarred. Data contains testing data along yaml catalogs that need to be updated to be compatible with both Intake versions 0.7 and 2.0+. See #415

To have successful tests we need

I don't particularly have access to such data.

After the files have been modified, we can re run the failed CI - tests. Once all CI jobs have passed, we can merge and close #415

Mikejmnez commented 7 months ago

Thanks Tom. We still need to fix the catalog yaml files in https://zenodo.org/record/5832607/ to allow CI jobs to run successfully with the new Intake version > 2.0. It looks like these belong to @malmans2

Specifically, we need to remove/comment the follow lines within the intake catalogs.

plugins:
  source:
    - module: intake_xarray

The files to be modified are: catalog_ECCO.yaml, catalog_xarray.yaml and hycom_test.yaml.

Once the edits to these three files are done, I can re-run the CI jobs and they should all pass with the new intake version (while remain backwards compatible with versions <=0.7). @malmans2 can you help us with that?

Have a good weekend!

Mikejmnez commented 7 months ago

or perhaps the best course of action is for you @ThomasHaine to create a zenodo entry with OceanSpy's updated test Data (tar format), since you are going to provide long term support.

Mikejmnez commented 7 months ago

The error is due to a deprecation unrelated to this PR/issue. See #421

MaceKuailv commented 7 months ago

421 seems to be an annoying CI issue. Does changing the Data file solves that as well?

I guess we can wait a few days for @malmans2. It is always nice to make the different versions of the test dataset consistent.

Mikejmnez commented 7 months ago

Hey @MaceKuailv ! The test data has been changed accordingly and this PR point to its correct location. @malmans2 created an OceanSpy community and that is where the data lives now. The issues are unrelated to this PR.

For some reason this PR did not include the new micromamba specifications but #421 did! (you can see I also made the changes to gihubworkflows/ci.yaml here). But I am pretty sure that merging #421 will allow all other PRs to pass (likely upon merging).

So, everything is ready to be merged, beginning with #421

Mikejmnez commented 7 months ago

Test data has been updated after some silly hiccups. Tests run well locally. But I am seeing some local test failure with test_open_oceandataset.from_catalog and the entries: xmitgcm_iters and xmitgcm_no_iters. This I need to double check. Should have some time in the weekend. This is low impact/low priority. I will designate this branch a draft in the meantime.

The conflict is a minor issue.

MaceKuailv commented 7 months ago

Good to know that the test dataset is in good shape. I don't know anything about intake, feel free to merge it anytime.

Mikejmnez commented 7 months ago

will close. PR #423 takes care of business...