Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intake conversion Cross-contour_transport #356

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

rbeucher
Copy link
Contributor

@rbeucher rbeucher commented Jun 6, 2024

Following the discussion in issue #313, we propose converting the recipes to use Intake, given that the Cookbook is no longer supported and the ACCESS-NRI Intake catalog is now available.

A few months ago, @max-anu began working on this transition. This pull request contains the changes @max-anu made to the notebook specified in the title.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@taimoorsohail taimoorsohail requested review from taimoorsohail and removed request for taimoorsohail July 1, 2024 03:43
@navidcy
Copy link
Collaborator

navidcy commented Jul 9, 2024

@taimoorsohail just checking: the Along_isobath_average.ipynb was pushed in this branch on purpose or was is supposed to go in #416 only?

@taimoorsohail
Copy link
Collaborator

I thought I pushed from taimoorsohail:main to Cosima-recipes:main? Either way, no, I wasn't trying to push in this branch. I was reviewing this PR request but there are significant bugs in the Cross-contour_transport code (not related to this PR - they are well documented in other issues), so I think @adele-morrison is ironing out those bugs.

@navidcy
Copy link
Collaborator

navidcy commented Jul 9, 2024

Your local clone must have had some edits from previous things you'd be working on and then you did something like git add * or git commit -am"message", either of which adds all the modifications to the commit.

That's a guess. I'll sort it out -- don't worry! :)

@navidcy
Copy link
Collaborator

navidcy commented Jul 9, 2024

I thought I pushed from taimoorsohail:main to cosima-recipes:main

I don't understand exactly what do you mean. Nobody is allowed to push to cosima-recipes:main, you can only make a PR to request merging changes to cosima-recipes:main.

This PR was opened from a branch of cosima-recipes. So to add commits to that PR you need to clone the main cosima-recipes branch, do git checkout INTAKE_Cross-contour_transport and then push commits to that branch.

Anyway I fixed it... I'm just trying to make it easier for other times but I'm not sure if I'm explaining myself properly. [I do admit that it's early am, I'm jet lagged and coffee shops are closed where I currently am! :)]

@marc-white
Copy link
Contributor

marc-white commented Sep 11, 2024

I got about halfway through merging the updates from the trunk (PR #423) yesterday, but now can't continue without Gadi access. Whilst doing that, I identified a potential issue with the Intake catalogue of the dataset being used: ACCESS-NRI/access-nri-intake-catalog#192

@marc-white
Copy link
Contributor

I've completed merging the latest main into this branch, and completed the intake conversion at the same time (I think). As part of that, I've added a small section on chunking the data; for the data volume in use, I doubt it makes a lot of difference, but I think it's a useful example. @navidcy I'll add you as a reviewer; please let me know if there's anyone else I should add.

@marc-white marc-white requested a review from navidcy September 16, 2024 06:26
@navidcy
Copy link
Collaborator

navidcy commented Sep 16, 2024

Actually could you request review from somebody else? There are many COSIMAers who would be willing to contribute.

@marc-white marc-white requested review from adele-morrison and removed request for navidcy September 16, 2024 06:43
@marc-white
Copy link
Contributor

@adele-morrison are you in a position to be able to review this, and if not, could you suggest someone who is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants