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.

Sign up for GitHub

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

Jump to bottom

add reset_encoding to dataset/dataarray/variable #7689

Merged
merged 5 commits into from
Mar 30, 2023
Merged

add reset_encoding to dataset/dataarray/variable #7689

merged 5 commits into from
Mar 30, 2023

Conversation

jhamman
Copy link
Member

@jhamman jhamman commented Mar 27, 2023

@jhamman jhamman requested a review from keewis March 27, 2023 22:35
keewis
keewis approved these changes Mar 28, 2023
View reviewed changes
Copy link
Collaborator

@keewis keewis left a comment

Choose a reason for hiding this comment

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

I was about to suggest naming it clear_encoding, but dict.clear works in-place, which I don't think we want to follow.

However, given #5336 (comment) we should probably discuss the general strategy in the meeting tomorrow before merging.

doc/user-guide/io.rst Outdated Show resolved Hide resolved
@dcherian
Copy link
Contributor

I think we can merge regardless of #5082. This is pretty useful right now

@jhamman jhamman added the plan to merge Final call for comments label Mar 28, 2023
@dcherian
Copy link
Contributor

In before a user asks for it, should we allow deleting only certain keys like dtype for example?

@jhamman
Copy link
Member Author

jhamman commented Mar 30, 2023

In before a user asks for it, should we allow deleting only certain keys like dtype for example?

Not opposed to this but do you think we'll really be asked for this? What sort of API do you envision?

@dcherian
Copy link
Contributor

ds.reset_encoding(keys=["dtype", "chunks"])

I agree that it may not be necessary.

@jhamman
Copy link
Member Author

jhamman commented Mar 30, 2023

I see how we could do that on the variable but its a bit messy on the Dataset/DataArray. I suggest we wait until this is a requested feature.

@dcherian dcherian merged commit 850156c into pydata:main Mar 30, 2023
@dcherian
Copy link
Contributor

Sounds good to me! Thanks @jhamman

@jhamman jhamman deleted the feature/reset_encoding branch March 30, 2023 21:28
macovskym added a commit to CAnBioNet/TkNA that referenced this pull request Dec 9, 2023
@macovskym
Add workaround to fix to_netcdf sometimes failing on filteredData
c7940b8 
The failure is a result of an xarray bug that can occur after subsetting
data that was itself loaded from netcdf.
See pydata/xarray#6352 for the issue and
pydata/xarray#7689 for the fix used to create
the workaround.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keewis keewis keewis approved these changes

Assignees
No one assigned
Labels
plan to merge Final call for comments
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add reset_encoding to Dataset and DataArray objects
3 participants
@jhamman @dcherian @keewis