Intake conversion Regridding example; take #2

[navidcy]

I tried to work with #366 but I couldn't resolve the merge conflicts... So I opened this PR to convert the Regridding example to use Intake instead of cookbook.

Also this PR deals with #418 regarding the Regridding example.

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[navidcy]

Took the opportunity to rename/retitle the notebook as per discussion in #412

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:56Z ----------------------------------------------------------------

Line #1.    import pandas as pd

This isn't used

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:56Z ----------------------------------------------------------------

Line #5.    ssh_1 = ds['sea_level'].sel(time=slice('2000-01-01', '2001-12-31')).cf.chunk({'time': 'auto', 'longitude': -1, 'latitude': -1})

I suggest we just .load() rather than rechunk ...

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:57Z ----------------------------------------------------------------

Line #5.    ssh_025 = ds['sea_level'].sel(time=slice('2000-01-01', '2001-12-31')).cf.chunk({'time': 'auto', 'longitude': -1, 'latitude': -1})

I suggest we just .load() rather than rechunk ...

_navidcy commented on 2024-07-09T12:19:12Z_ ----------------------------------------------------------------

I left the chunck as it might be needed if you load a larger data array that you can't .load().What do you think?

_anton-seaice commented on 2024-07-10T00:27:59Z_ ----------------------------------------------------------------

It doesn't appear to have made any difference to the times (or perhaps its slower?) so without .load is fine

_navidcy commented on 2024-07-11T14:20:22Z_ ----------------------------------------------------------------

OK, done

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:58Z ----------------------------------------------------------------

Line #5.    ssh_010 = ds['sea_level'].sel(time=slice('2000-01-01', '2001-12-31')).cf.chunk({'time': 'auto', 'longitude': -1, 'latitude': -1})

I suggest we just .load() rather than rechunk ...

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:58Z ----------------------------------------------------------------

Line #4.    regridder_1degACCESSOM2_1deg = xesmf.Regridder(ds_in_1deg, ds_out, 'bilinear', periodic=True,

Can you run black or something on this notebook? The formatting is kind of messy

_navidcy commented on 2024-07-09T12:19:34Z_ ----------------------------------------------------------------

I don't know how to do that! Can you do it perhaps?

_navidcy commented on 2024-07-09T12:20:00Z_ ----------------------------------------------------------------

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

_anton-seaice commented on 2024-07-10T00:27:17Z_ ----------------------------------------------------------------

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

_navidcy commented on 2024-07-11T14:20:55Z_ ----------------------------------------------------------------

hm... I can't... can we leave this for some other time?

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

Nevermind .. it does need dask.

navidcy commented 4 months ago

@anton-seaice, thanks for the comments!I think I addressed all of them!

navidcy commented 4 months ago

I think this notebook might not need dask at all actually.

But what if you load more than one snapshot of fields you wanna regrid?

navidcy commented 4 months ago

I left the chunck as it might be needed if you load a larger data array that you can't .load().What do you think?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I don't know how to do that! Can you do it perhaps?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

It doesn't appear to have made any difference to the times (or perhaps its slower?) so without .load is fine

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:28:48Z ----------------------------------------------------------------

Line #1.    %%time

There doesn't appear to be a need to time these three cells

_navidcy commented on 2024-07-11T14:21:09Z_ ----------------------------------------------------------------

deal

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:32:24Z ----------------------------------------------------------------

Maybe change 'the same 1 degree longitude-latitude grid. '

to "a 1 degree longitude-latitude grid with regular spacing." ?

Maybe "In this example, " is better than "In particular, "

navidcy commented on 2024-07-11T14:19:58Z ----------------------------------------------------------------

Done

anton-seaice commented 4 months ago

Apologies for the double-review. This is very close :)

navidcy commented 4 months ago

@anton-seaice, review is a continuing process! we expect review comments until the community is happy -- there is no quota on reviews!

navidcy commented 4 months ago

@anton-seaice I'll skip black. I get this:

(base) nc3020@gadi-login-07:~/gdata/cosima-recipes$ black Examples/Horizontal_Regridding.ipynb
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running ``pip install "black[jupyter]"``
No Python files are present to be formatted. Nothing to do 😴

and even after installing via pip install "black[jupyter]" --user I still get the same error...

navidcy commented 4 months ago

I did everything except the black formatting.

We could set up a GitHub action to apply black to all notebooks? I'm not sure if this is a good idea... Sometimes black makes things less pretty. It's also subjective...

navidcy commented 4 months ago

Done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

OK, done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

hm... I can't... can we leave this for some other time?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

deal

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-11T23:12:12Z ----------------------------------------------------------------

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

navidcy commented on 2024-07-19T03:08:36Z ----------------------------------------------------------------

done

navidcy commented 4 months ago

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

Are you sure about this?

I thought these warnings are related to #150 and #212 because some coords are missing from the 1deg grid saved in ik11. But I may be wrong... I was merely guessing tbh.

anton-seaice commented 4 months ago

Im moderately confident its because of these lines:

ssh_1 = ssh_1.rename(
    {"xt_ocean": "x", "yt_ocean": "y", "geolon_t": "longitude", "geolat_t": "latitude"}
)

but the cf-attributes still refer to 'geolon_t' and 'geolat_t', which don't exist, so theres an error ?

navidcy commented 4 months ago

done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

@anton-seaice I believe this is ready

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:56Z ----------------------------------------------------------------

Line #5.    ssh_1 = ds['sea_level'].sel(time=slice('2000-01-01', '2001-12-31')).cf.chunk({'time': 'auto', 'longitude': -1, 'latitude': -1})

I suggest we just .load() rather than rechunk ...

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:57Z ----------------------------------------------------------------

Line #5.    ssh_025 = ds['sea_level'].sel(time=slice('2000-01-01', '2001-12-31')).cf.chunk({'time': 'auto', 'longitude': -1, 'latitude': -1})

I suggest we just .load() rather than rechunk ...

_navidcy commented on 2024-07-09T12:19:12Z_ ----------------------------------------------------------------

I left the chunck as it might be needed if you load a larger data array that you can't .load().What do you think?

_anton-seaice commented on 2024-07-10T00:27:59Z_ ----------------------------------------------------------------

It doesn't appear to have made any difference to the times (or perhaps its slower?) so without .load is fine

_navidcy commented on 2024-07-11T14:20:22Z_ ----------------------------------------------------------------

OK, done

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:58Z ----------------------------------------------------------------

Line #5.    ssh_010 = ds['sea_level'].sel(time=slice('2000-01-01', '2001-12-31')).cf.chunk({'time': 'auto', 'longitude': -1, 'latitude': -1})

I suggest we just .load() rather than rechunk ...

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:58Z ----------------------------------------------------------------

Line #4.    regridder_1degACCESSOM2_1deg = xesmf.Regridder(ds_in_1deg, ds_out, 'bilinear', periodic=True,

Can you run black or something on this notebook? The formatting is kind of messy

_navidcy commented on 2024-07-09T12:19:34Z_ ----------------------------------------------------------------

I don't know how to do that! Can you do it perhaps?

_navidcy commented on 2024-07-09T12:20:00Z_ ----------------------------------------------------------------

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

_anton-seaice commented on 2024-07-10T00:27:17Z_ ----------------------------------------------------------------

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

_navidcy commented on 2024-07-11T14:20:55Z_ ----------------------------------------------------------------

hm... I can't... can we leave this for some other time?

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

Nevermind .. it does need dask.

navidcy commented 4 months ago

@anton-seaice, thanks for the comments!I think I addressed all of them!

navidcy commented 4 months ago

I think this notebook might not need dask at all actually.

But what if you load more than one snapshot of fields you wanna regrid?

navidcy commented 4 months ago

I left the chunck as it might be needed if you load a larger data array that you can't .load().What do you think?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I don't know how to do that! Can you do it perhaps?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

It doesn't appear to have made any difference to the times (or perhaps its slower?) so without .load is fine

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:28:48Z ----------------------------------------------------------------

Line #1.    %%time

There doesn't appear to be a need to time these three cells

_navidcy commented on 2024-07-11T14:21:09Z_ ----------------------------------------------------------------

deal

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:32:24Z ----------------------------------------------------------------

Maybe change 'the same 1 degree longitude-latitude grid. '

to "a 1 degree longitude-latitude grid with regular spacing." ?

Maybe "In this example, " is better than "In particular, "

navidcy commented on 2024-07-11T14:19:58Z ----------------------------------------------------------------

Done

anton-seaice commented 4 months ago

Apologies for the double-review. This is very close :)

navidcy commented 4 months ago

@anton-seaice, review is a continuing process! we expect review comments until the community is happy -- there is no quota on reviews!

navidcy commented 4 months ago

@anton-seaice I'll skip black. I get this:

(base) nc3020@gadi-login-07:~/gdata/cosima-recipes$ black Examples/Horizontal_Regridding.ipynb
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running ``pip install "black[jupyter]"``
No Python files are present to be formatted. Nothing to do 😴

and even after installing via pip install "black[jupyter]" --user I still get the same error...

navidcy commented 4 months ago

I did everything except the black formatting.

We could set up a GitHub action to apply black to all notebooks? I'm not sure if this is a good idea... Sometimes black makes things less pretty. It's also subjective...

navidcy commented 4 months ago

Done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

OK, done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

hm... I can't... can we leave this for some other time?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

deal

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-11T23:12:12Z ----------------------------------------------------------------

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

navidcy commented on 2024-07-19T03:08:36Z ----------------------------------------------------------------

done

navidcy commented 4 months ago

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

Are you sure about this?

I thought these warnings are related to #150 and #212 because some coords are missing from the 1deg grid saved in ik11. But I may be wrong... I was merely guessing tbh.

anton-seaice commented 4 months ago

Im moderately confident its because of these lines:

ssh_1 = ssh_1.rename(
    {"xt_ocean": "x", "yt_ocean": "y", "geolon_t": "longitude", "geolat_t": "latitude"}
)

but the cf-attributes still refer to 'geolon_t' and 'geolat_t', which don't exist, so theres an error ?

navidcy commented 4 months ago

done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

@anton-seaice I believe this is ready

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:57Z ----------------------------------------------------------------

Line #5.    ssh_025 = ds['sea_level'].sel(time=slice('2000-01-01', '2001-12-31')).cf.chunk({'time': 'auto', 'longitude': -1, 'latitude': -1})

I suggest we just .load() rather than rechunk ...

_navidcy commented on 2024-07-09T12:19:12Z_ ----------------------------------------------------------------

I left the chunck as it might be needed if you load a larger data array that you can't .load().What do you think?

_anton-seaice commented on 2024-07-10T00:27:59Z_ ----------------------------------------------------------------

It doesn't appear to have made any difference to the times (or perhaps its slower?) so without .load is fine

_navidcy commented on 2024-07-11T14:20:22Z_ ----------------------------------------------------------------

OK, done

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:58Z ----------------------------------------------------------------

Line #5.    ssh_010 = ds['sea_level'].sel(time=slice('2000-01-01', '2001-12-31')).cf.chunk({'time': 'auto', 'longitude': -1, 'latitude': -1})

I suggest we just .load() rather than rechunk ...

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:58Z ----------------------------------------------------------------

Line #4.    regridder_1degACCESSOM2_1deg = xesmf.Regridder(ds_in_1deg, ds_out, 'bilinear', periodic=True,

Can you run black or something on this notebook? The formatting is kind of messy

_navidcy commented on 2024-07-09T12:19:34Z_ ----------------------------------------------------------------

I don't know how to do that! Can you do it perhaps?

_navidcy commented on 2024-07-09T12:20:00Z_ ----------------------------------------------------------------

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

_anton-seaice commented on 2024-07-10T00:27:17Z_ ----------------------------------------------------------------

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

_navidcy commented on 2024-07-11T14:20:55Z_ ----------------------------------------------------------------

hm... I can't... can we leave this for some other time?

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

Nevermind .. it does need dask.

navidcy commented 4 months ago

@anton-seaice, thanks for the comments!I think I addressed all of them!

navidcy commented 4 months ago

I think this notebook might not need dask at all actually.

But what if you load more than one snapshot of fields you wanna regrid?

navidcy commented 4 months ago

I left the chunck as it might be needed if you load a larger data array that you can't .load().What do you think?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I don't know how to do that! Can you do it perhaps?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

It doesn't appear to have made any difference to the times (or perhaps its slower?) so without .load is fine

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:28:48Z ----------------------------------------------------------------

Line #1.    %%time

There doesn't appear to be a need to time these three cells

_navidcy commented on 2024-07-11T14:21:09Z_ ----------------------------------------------------------------

deal

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:32:24Z ----------------------------------------------------------------

Maybe change 'the same 1 degree longitude-latitude grid. '

to "a 1 degree longitude-latitude grid with regular spacing." ?

Maybe "In this example, " is better than "In particular, "

navidcy commented on 2024-07-11T14:19:58Z ----------------------------------------------------------------

Done

anton-seaice commented 4 months ago

Apologies for the double-review. This is very close :)

navidcy commented 4 months ago

@anton-seaice, review is a continuing process! we expect review comments until the community is happy -- there is no quota on reviews!

navidcy commented 4 months ago

@anton-seaice I'll skip black. I get this:

(base) nc3020@gadi-login-07:~/gdata/cosima-recipes$ black Examples/Horizontal_Regridding.ipynb
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running ``pip install "black[jupyter]"``
No Python files are present to be formatted. Nothing to do 😴

and even after installing via pip install "black[jupyter]" --user I still get the same error...

navidcy commented 4 months ago

I did everything except the black formatting.

We could set up a GitHub action to apply black to all notebooks? I'm not sure if this is a good idea... Sometimes black makes things less pretty. It's also subjective...

navidcy commented 4 months ago

Done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

OK, done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

hm... I can't... can we leave this for some other time?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

deal

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-11T23:12:12Z ----------------------------------------------------------------

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

navidcy commented on 2024-07-19T03:08:36Z ----------------------------------------------------------------

done

navidcy commented 4 months ago

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

Are you sure about this?

I thought these warnings are related to #150 and #212 because some coords are missing from the 1deg grid saved in ik11. But I may be wrong... I was merely guessing tbh.

anton-seaice commented 4 months ago

Im moderately confident its because of these lines:

ssh_1 = ssh_1.rename(
    {"xt_ocean": "x", "yt_ocean": "y", "geolon_t": "longitude", "geolat_t": "latitude"}
)

but the cf-attributes still refer to 'geolon_t' and 'geolat_t', which don't exist, so theres an error ?

navidcy commented 4 months ago

done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

@anton-seaice I believe this is ready

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:58Z ----------------------------------------------------------------

Line #5.    ssh_010 = ds['sea_level'].sel(time=slice('2000-01-01', '2001-12-31')).cf.chunk({'time': 'auto', 'longitude': -1, 'latitude': -1})

I suggest we just .load() rather than rechunk ...

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:58Z ----------------------------------------------------------------

Line #4.    regridder_1degACCESSOM2_1deg = xesmf.Regridder(ds_in_1deg, ds_out, 'bilinear', periodic=True,

Can you run black or something on this notebook? The formatting is kind of messy

_navidcy commented on 2024-07-09T12:19:34Z_ ----------------------------------------------------------------

I don't know how to do that! Can you do it perhaps?

_navidcy commented on 2024-07-09T12:20:00Z_ ----------------------------------------------------------------

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

_anton-seaice commented on 2024-07-10T00:27:17Z_ ----------------------------------------------------------------

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

_navidcy commented on 2024-07-11T14:20:55Z_ ----------------------------------------------------------------

hm... I can't... can we leave this for some other time?

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

Nevermind .. it does need dask.

navidcy commented 4 months ago

@anton-seaice, thanks for the comments!I think I addressed all of them!

navidcy commented 4 months ago

I think this notebook might not need dask at all actually.

But what if you load more than one snapshot of fields you wanna regrid?

navidcy commented 4 months ago

I left the chunck as it might be needed if you load a larger data array that you can't .load().What do you think?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I don't know how to do that! Can you do it perhaps?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

It doesn't appear to have made any difference to the times (or perhaps its slower?) so without .load is fine

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:28:48Z ----------------------------------------------------------------

Line #1.    %%time

There doesn't appear to be a need to time these three cells

_navidcy commented on 2024-07-11T14:21:09Z_ ----------------------------------------------------------------

deal

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:32:24Z ----------------------------------------------------------------

Maybe change 'the same 1 degree longitude-latitude grid. '

to "a 1 degree longitude-latitude grid with regular spacing." ?

Maybe "In this example, " is better than "In particular, "

navidcy commented on 2024-07-11T14:19:58Z ----------------------------------------------------------------

Done

anton-seaice commented 4 months ago

Apologies for the double-review. This is very close :)

navidcy commented 4 months ago

@anton-seaice, review is a continuing process! we expect review comments until the community is happy -- there is no quota on reviews!

navidcy commented 4 months ago

@anton-seaice I'll skip black. I get this:

(base) nc3020@gadi-login-07:~/gdata/cosima-recipes$ black Examples/Horizontal_Regridding.ipynb
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running ``pip install "black[jupyter]"``
No Python files are present to be formatted. Nothing to do 😴

and even after installing via pip install "black[jupyter]" --user I still get the same error...

navidcy commented 4 months ago

I did everything except the black formatting.

We could set up a GitHub action to apply black to all notebooks? I'm not sure if this is a good idea... Sometimes black makes things less pretty. It's also subjective...

navidcy commented 4 months ago

Done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

OK, done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

hm... I can't... can we leave this for some other time?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

deal

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-11T23:12:12Z ----------------------------------------------------------------

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

navidcy commented on 2024-07-19T03:08:36Z ----------------------------------------------------------------

done

navidcy commented 4 months ago

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

Are you sure about this?

I thought these warnings are related to #150 and #212 because some coords are missing from the 1deg grid saved in ik11. But I may be wrong... I was merely guessing tbh.

anton-seaice commented 4 months ago

Im moderately confident its because of these lines:

ssh_1 = ssh_1.rename(
    {"xt_ocean": "x", "yt_ocean": "y", "geolon_t": "longitude", "geolat_t": "latitude"}
)

but the cf-attributes still refer to 'geolon_t' and 'geolat_t', which don't exist, so theres an error ?

navidcy commented 4 months ago

done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

@anton-seaice I believe this is ready

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-08T07:05:58Z ----------------------------------------------------------------

Line #4.    regridder_1degACCESSOM2_1deg = xesmf.Regridder(ds_in_1deg, ds_out, 'bilinear', periodic=True,

Can you run black or something on this notebook? The formatting is kind of messy

_navidcy commented on 2024-07-09T12:19:34Z_ ----------------------------------------------------------------

I don't know how to do that! Can you do it perhaps?

_navidcy commented on 2024-07-09T12:20:00Z_ ----------------------------------------------------------------

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

_anton-seaice commented on 2024-07-10T00:27:17Z_ ----------------------------------------------------------------

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

_navidcy commented on 2024-07-11T14:20:55Z_ ----------------------------------------------------------------

hm... I can't... can we leave this for some other time?

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

anton-seaice commented 4 months ago

I think this notebook might not need dask at all actually.

Nevermind .. it does need dask.

navidcy commented 4 months ago

@anton-seaice, thanks for the comments!I think I addressed all of them!

navidcy commented 4 months ago

I think this notebook might not need dask at all actually.

But what if you load more than one snapshot of fields you wanna regrid?

navidcy commented 4 months ago

I left the chunck as it might be needed if you load a larger data array that you can't .load().What do you think?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I don't know how to do that! Can you do it perhaps?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

---

View entire conversation on ReviewNB

anton-seaice commented 4 months ago

It doesn't appear to have made any difference to the times (or perhaps its slower?) so without .load is fine

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:28:48Z ----------------------------------------------------------------

Line #1.    %%time

There doesn't appear to be a need to time these three cells

_navidcy commented on 2024-07-11T14:21:09Z_ ----------------------------------------------------------------

deal

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:32:24Z ----------------------------------------------------------------

Maybe change 'the same 1 degree longitude-latitude grid. '

to "a 1 degree longitude-latitude grid with regular spacing." ?

Maybe "In this example, " is better than "In particular, "

navidcy commented on 2024-07-11T14:19:58Z ----------------------------------------------------------------

Done

anton-seaice commented 4 months ago

Apologies for the double-review. This is very close :)

navidcy commented 4 months ago

@anton-seaice, review is a continuing process! we expect review comments until the community is happy -- there is no quota on reviews!

navidcy commented 4 months ago

@anton-seaice I'll skip black. I get this:

(base) nc3020@gadi-login-07:~/gdata/cosima-recipes$ black Examples/Horizontal_Regridding.ipynb
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running ``pip install "black[jupyter]"``
No Python files are present to be formatted. Nothing to do 😴

and even after installing via pip install "black[jupyter]" --user I still get the same error...

navidcy commented 4 months ago

I did everything except the black formatting.

We could set up a GitHub action to apply black to all notebooks? I'm not sure if this is a good idea... Sometimes black makes things less pretty. It's also subjective...

navidcy commented 4 months ago

Done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

OK, done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

hm... I can't... can we leave this for some other time?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

deal

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-11T23:12:12Z ----------------------------------------------------------------

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

navidcy commented on 2024-07-19T03:08:36Z ----------------------------------------------------------------

done

navidcy commented 4 months ago

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

Are you sure about this?

I thought these warnings are related to #150 and #212 because some coords are missing from the 1deg grid saved in ik11. But I may be wrong... I was merely guessing tbh.

anton-seaice commented 4 months ago

Im moderately confident its because of these lines:

ssh_1 = ssh_1.rename(
    {"xt_ocean": "x", "yt_ocean": "y", "geolon_t": "longitude", "geolat_t": "latitude"}
)

but the cf-attributes still refer to 'geolon_t' and 'geolat_t', which don't exist, so theres an error ?

navidcy commented 4 months ago

done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

@anton-seaice I believe this is ready

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[anton-seaice]

I think this notebook might not need dask at all actually.

[anton-seaice]

I think this notebook might not need dask at all actually.

Nevermind .. it does need dask.

[navidcy]

@anton-seaice, thanks for the comments!I think I addressed all of them!

[navidcy]

I think this notebook might not need dask at all actually.

But what if you load more than one snapshot of fields you wanna regrid?

[navidcy]

I left the chunck as it might be needed if you load a larger data array that you can't .load().What do you think?

---

View entire conversation on ReviewNB

[navidcy]

I don't know how to do that! Can you do it perhaps?

---

View entire conversation on ReviewNB

[navidcy]

I know how to run black on .py scripts... Is it the same but black notebook.ipynb?

---

View entire conversation on ReviewNB

[anton-seaice]

Yes correct - its in conda/analysis3 so you should be able to run that in the ARE terminal or a Gadi login

---

View entire conversation on ReviewNB

[anton-seaice]

It doesn't appear to have made any difference to the times (or perhaps its slower?) so without .load is fine

---

View entire conversation on ReviewNB

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:28:48Z ----------------------------------------------------------------

Line #1.    %%time

There doesn't appear to be a need to time these three cells

_navidcy commented on 2024-07-11T14:21:09Z_ ----------------------------------------------------------------

deal

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:32:24Z ----------------------------------------------------------------

Maybe change 'the same 1 degree longitude-latitude grid. '

to "a 1 degree longitude-latitude grid with regular spacing." ?

Maybe "In this example, " is better than "In particular, "

navidcy commented on 2024-07-11T14:19:58Z ----------------------------------------------------------------

Done

anton-seaice commented 4 months ago

Apologies for the double-review. This is very close :)

navidcy commented 4 months ago

@anton-seaice, review is a continuing process! we expect review comments until the community is happy -- there is no quota on reviews!

navidcy commented 4 months ago

@anton-seaice I'll skip black. I get this:

(base) nc3020@gadi-login-07:~/gdata/cosima-recipes$ black Examples/Horizontal_Regridding.ipynb
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running ``pip install "black[jupyter]"``
No Python files are present to be formatted. Nothing to do 😴

and even after installing via pip install "black[jupyter]" --user I still get the same error...

navidcy commented 4 months ago

I did everything except the black formatting.

We could set up a GitHub action to apply black to all notebooks? I'm not sure if this is a good idea... Sometimes black makes things less pretty. It's also subjective...

navidcy commented 4 months ago

Done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

OK, done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

hm... I can't... can we leave this for some other time?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

deal

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-11T23:12:12Z ----------------------------------------------------------------

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

navidcy commented on 2024-07-19T03:08:36Z ----------------------------------------------------------------

done

navidcy commented 4 months ago

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

Are you sure about this?

I thought these warnings are related to #150 and #212 because some coords are missing from the 1deg grid saved in ik11. But I may be wrong... I was merely guessing tbh.

anton-seaice commented 4 months ago

Im moderately confident its because of these lines:

ssh_1 = ssh_1.rename(
    {"xt_ocean": "x", "yt_ocean": "y", "geolon_t": "longitude", "geolat_t": "latitude"}
)

but the cf-attributes still refer to 'geolon_t' and 'geolat_t', which don't exist, so theres an error ?

navidcy commented 4 months ago

done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

@anton-seaice I believe this is ready

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-10T00:32:24Z ----------------------------------------------------------------

Maybe change 'the same 1 degree longitude-latitude grid. '

to "a 1 degree longitude-latitude grid with regular spacing." ?

Maybe "In this example, " is better than "In particular, "

navidcy commented on 2024-07-11T14:19:58Z ----------------------------------------------------------------

Done

anton-seaice commented 4 months ago

Apologies for the double-review. This is very close :)

navidcy commented 4 months ago

@anton-seaice, review is a continuing process! we expect review comments until the community is happy -- there is no quota on reviews!

navidcy commented 4 months ago

@anton-seaice I'll skip black. I get this:

(base) nc3020@gadi-login-07:~/gdata/cosima-recipes$ black Examples/Horizontal_Regridding.ipynb
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running ``pip install "black[jupyter]"``
No Python files are present to be formatted. Nothing to do 😴

and even after installing via pip install "black[jupyter]" --user I still get the same error...

navidcy commented 4 months ago

I did everything except the black formatting.

We could set up a GitHub action to apply black to all notebooks? I'm not sure if this is a good idea... Sometimes black makes things less pretty. It's also subjective...

navidcy commented 4 months ago

Done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

OK, done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

hm... I can't... can we leave this for some other time?

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

deal

---

View entire conversation on ReviewNB

review-notebook-app[bot] commented 4 months ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-11T23:12:12Z ----------------------------------------------------------------

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

navidcy commented on 2024-07-19T03:08:36Z ----------------------------------------------------------------

done

navidcy commented 4 months ago

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

Are you sure about this?

I thought these warnings are related to #150 and #212 because some coords are missing from the 1deg grid saved in ik11. But I may be wrong... I was merely guessing tbh.

anton-seaice commented 4 months ago

Im moderately confident its because of these lines:

ssh_1 = ssh_1.rename(
    {"xt_ocean": "x", "yt_ocean": "y", "geolon_t": "longitude", "geolat_t": "latitude"}
)

but the cf-attributes still refer to 'geolon_t' and 'geolat_t', which don't exist, so theres an error ?

navidcy commented 4 months ago

done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

@anton-seaice I believe this is ready

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[anton-seaice]

Apologies for the double-review. This is very close :)

[navidcy]

@anton-seaice, review is a continuing process! we expect review comments until the community is happy -- there is no quota on reviews!

[navidcy]

@anton-seaice I'll skip black. I get this:

(base) nc3020@gadi-login-07:~/gdata/cosima-recipes$ black Examples/Horizontal_Regridding.ipynb
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running ``pip install "black[jupyter]"``
No Python files are present to be formatted. Nothing to do 😴

and even after installing via pip install "black[jupyter]" --user I still get the same error...

[navidcy]

I did everything except the black formatting.

We could set up a GitHub action to apply black to all notebooks? I'm not sure if this is a good idea... Sometimes black makes things less pretty. It's also subjective...

[navidcy]

Done

---

View entire conversation on ReviewNB

[navidcy]

OK, done

---

View entire conversation on ReviewNB

[navidcy]

hm... I can't... can we leave this for some other time?

---

View entire conversation on ReviewNB

[navidcy]

deal

---

View entire conversation on ReviewNB

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-07-11T23:12:12Z ----------------------------------------------------------------

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

navidcy commented on 2024-07-19T03:08:36Z ----------------------------------------------------------------

done

navidcy commented 4 months ago

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

Are you sure about this?

I thought these warnings are related to #150 and #212 because some coords are missing from the 1deg grid saved in ik11. But I may be wrong... I was merely guessing tbh.

anton-seaice commented 4 months ago

Im moderately confident its because of these lines:

ssh_1 = ssh_1.rename(
    {"xt_ocean": "x", "yt_ocean": "y", "geolon_t": "longitude", "geolat_t": "latitude"}
)

but the cf-attributes still refer to 'geolon_t' and 'geolat_t', which don't exist, so theres an error ?

navidcy commented 4 months ago

done

---

View entire conversation on ReviewNB

navidcy commented 4 months ago

@anton-seaice I believe this is ready

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[navidcy]

We can get rid of the geolon/ geolat warnings by either not renaming 'geolon' to longitude and 'geolat' to latitde, or by updating the attributes when we do? Thoughts?

xesmf should understand cf attributes, so i think we can leave the names as geolon/geolat and it will know what to do

Are you sure about this?

I thought these warnings are related to #150 and #212 because some coords are missing from the 1deg grid saved in ik11. But I may be wrong... I was merely guessing tbh.

[anton-seaice]

Im moderately confident its because of these lines:

ssh_1 = ssh_1.rename(
    {"xt_ocean": "x", "yt_ocean": "y", "geolon_t": "longitude", "geolat_t": "latitude"}
)

but the cf-attributes still refer to 'geolon_t' and 'geolat_t', which don't exist, so theres an error ?

[navidcy]

done

---

View entire conversation on ReviewNB

[navidcy]

@anton-seaice I believe this is ready