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

Aggregation control still non-optional #568

Closed
aulemahal opened this issue Feb 3, 2023 · 0 comments · Fixed by #569
Closed

Aggregation control still non-optional #568

aulemahal opened this issue Feb 3, 2023 · 0 comments · Fixed by #569

Comments

@aulemahal
Copy link
Contributor

Almost the same issue as #549. PR #551 fixed the pydantic model issue, however the code itself expected an aggregation_control object to exist, so the bug remains.

First, I'm not sure if aggregation_control should be optional. When it is None, the keys of the dataset are simply all the columns joined together, including the path. There is also no variable_column_name and thus no derived variable registry and ESMDataSource can't select the expected variable.

I have a version where (AFAIK) intake_esm now works with aggregation_control=None, but I think an alternative would be to make it required. What do people think?

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 a pull request may close this issue.

1 participant