search

rom-py / rompy

Relocatable Ocean Modelling in PYthon (rompy) combines templated cookie-cutter model configuration with various xarray extensions to assist in the setup and evaluation of coastal ocean model
https://rom-py.github.io/rompy/
BSD 3-Clause "New" or "Revised" License
2 stars 9 forks source link

Edit AODN altimetry catalog #9

Closed rfonsecadasilva closed 3 years ago

rfonsecadasilva commented 3 years ago
pbranson commented 3 years ago

Actually is it possible for you to add a couple of tests too? Maybe a couple of pre-defined catalog entry requests where you know the length of the urlpath list and another where you know the length of the returned dataset.

rfonsecadasilva commented 3 years ago

Actually is it possible for you to add a couple of tests too? Maybe a couple of pre-defined catalog entry requests where you know the length of the urlpath list and another where you know the length of the returned dataset.

Sure, I have now these two new tests in test_intake. (For now, they are in a new commit only on my fork.)

pbranson commented 3 years ago

Thats good, thats how it works - any commits you add to your fork will come into this PR. That is why in general it is better to create a branch in your fork specific to the changes you want included in the PR

pbranson commented 3 years ago

You will also need to pull updates from main. I have fixed the github actions so that the tests get run on the PR

rfonsecadasilva commented 3 years ago

I added geopandas and merged upstream/main with your new changes.

pbranson commented 3 years ago

The tests are still failing - thats because there is a separate requirements file that is used to setup the testing environment. Probably there is a better way to configure this so that there isnt duplication, but you also need to add to: https://github.com/rfonsecadasilva/rompy/blob/main/scripts/ci/environment-pip.yml

To see your tests you can look here: https://github.com/rfonsecadasilva/rompy/actions

pbranson commented 3 years ago

And now the tests are running here so we should be able to monitor within this PR thread

rfonsecadasilva commented 3 years ago

The tests are still failing - thats because there is a separate requirements file that is used to setup the testing environment. Probably there is a better way to configure this so that there isnt duplication, but you also need to add to: https://github.com/rfonsecadasilva/rompy/blob/main/scripts/ci/environment-pip.yml

To see your tests you can look here: https://github.com/rfonsecadasilva/rompy/actions

Thanks, that's good to know. I've included geopandas and intake-geopandas and not the tests successfully passed on my fork.

And now the tests are running here so we should be able to monitor within this PR thread

I couldn't find new tests running here in the PR (maybe you first have to approve them before it starts testing?)

pbranson commented 3 years ago

Yeah that seemed to be because you hadnt made a commit to this repo before. I had to approve the CI run.

This all looks good - merging! 🎉