镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Add NextGEMS ICON model data source - #13

Merged
treigerm merged 5 commits into
mainfrom
nextgems
Mar 6, 2025
Merged

treigerm merged 5 commits into
mainfrom
nextgems

Conversation

@treigerm

@treigerm treigerm commented Mar 5, 2025

Copy link
Copy Markdown
Member

This is adding the functionality to download the publicly available NextGEMS ICON data. This can be used as a replacement of the DYAMOND datasets discussed in #10 . The NextGEMS simulations are using more recent model versions so are preferrable to DYAMOND anyway.

@treigerm
treigerm requested a review from juntyr March 5, 2025 16:18

@juntyr juntyr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM, I just had a few questions and small nits

Comment thread pyproject.toml
"dask~=2024.12.0",
"fsspec~=2024.10.0",
"healpy~=1.18.0",
"intake==0.7.0",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are the intake version bounds strictly pinned?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NextGEMS catalog at "https://data.nextgems-h2020.eu/online.yaml" does not seem to be compatible with any other version. The NextGEMS people I've talked to specifically mentioned that I need to use these version bounds (I don't have any more details though).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a comment here so we don't forget the reason later?

Comment thread pyproject.toml
"intake==0.7.0",
"intake-xarray==0.7.0",
"requests~=2.32.3",
"setuptools~=75.8.2",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are we using setuptools for?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intake-xarray is requiring setuptools. They probably haven't specified their Python version bounds precisely. So I needed to add setuptools to deal with a No module named 'distutils' error caused by the intake-xarray library.

https://stackoverflow.com/questions/69919970/no-module-named-distutils-util-but-distutils-is-installed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment here so we don't forget the reason later?

@juntyr juntyr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a very minor nit left, feel free to merge afterwards!

Comment thread pyproject.toml
"dask~=2024.12.0",
"fsspec~=2024.10.0",
"healpy~=1.18.0",
"intake==0.7.0",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a comment here so we don't forget the reason later?

Comment thread pyproject.toml
"intake==0.7.0",
"intake-xarray==0.7.0",
"requests~=2.32.3",
"setuptools~=75.8.2",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment here so we don't forget the reason later?

@treigerm

treigerm commented Mar 6, 2025

Copy link
Copy Markdown
Member Author

Great, thanks a lot for the review @juntyr ! Merging now.

@treigerm
treigerm merged commit 9f7f08e into main Mar 6, 2025
@treigerm
treigerm deleted the nextgems branch March 6, 2025 13:40
@treigerm treigerm mentioned this pull request Mar 6, 2025
3 of 4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants