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

Add to_collection method returning an Xcollection #405

Merged
merged 35 commits into from
Nov 28, 2021

Conversation

jukent
Copy link
Collaborator

@jukent jukent commented Nov 24, 2021

Change Summary

Updating to_dataset_dict to return an xcollection.Collection object.

Needed to add git+git://github.com/NCAR/xcollection.git to requirements
And wrap dictionaries in xc.Collection

There are some uses of the word "Collection" throughout the file that are not an xc.Collection, we might want to be very specific with our language throughout.

Tests seem to be unchanged by Collection vs Dictionary typing, maybe add an assert type test.

Related issue number

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable

@jukent
Copy link
Collaborator Author

jukent commented Nov 24, 2021

In the tests we have

)
def test_to_dataset_dict(path, query, xarray_open_kwargs):
    cat = intake.open_esm_datastore(path)
    cat_sub = cat.search(**query)
    _, ds = cat_sub.to_dataset_dict(xarray_open_kwargs=xarray_open_kwargs).popitem()
    assert 'member_id' in ds.dims
    assert len(ds.__dask_keys__()) > 0
    assert ds.time.encoding

But an xc.Collection won't have dims

@jukent
Copy link
Collaborator Author

jukent commented Nov 24, 2021

@kmpaul This is failing with "Error: The process '/usr/share/miniconda3/condabin/mamba' failed with exit code 1" which seems to be the same as from the ProjectPythia pull requests (related to the new mamba version?). I'm looking for how you fixed that in those repositories now.

ProjectPythia/projectpythia.github.io@8d5411e

Copy link
Collaborator

@kmpaul kmpaul left a comment

Choose a reason for hiding this comment

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

Error in protocol

requirements.txt Outdated Show resolved Hide resolved
@kmpaul kmpaul self-requested a review November 24, 2021 18:33
requirements.txt Outdated Show resolved Hide resolved
@kmpaul kmpaul changed the title Collection Add to_collection method returning an Xcollection Nov 24, 2021
intake_esm/core.py Outdated Show resolved Hide resolved
Co-authored-by: Anderson Banihirwe <[email protected]>
setup.py Outdated Show resolved Hide resolved
intake_esm/core.py Outdated Show resolved Hide resolved
Co-authored-by: Anderson Banihirwe <[email protected]>
@jukent jukent marked this pull request as ready for review November 24, 2021 20:08
@jukent jukent requested a review from andersy005 November 24, 2021 20:08
@andersy005 andersy005 added this to the Winter 2021 Release milestone Nov 24, 2021
Copy link
Member

@andersy005 andersy005 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this, @jukent! Looks great!

Copy link
Collaborator

@kmpaul kmpaul left a comment

Choose a reason for hiding this comment

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

Thanks, @jukent!

@andersy005 andersy005 merged commit 6128df6 into intake:main Nov 28, 2021
@jukent jukent deleted the collection branch November 29, 2021 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Collection object implementation from NCAR/xcollection in intake-esm
3 participants