Skip to content

Preserve attrs with coarsen#4360

Merged
dcherian merged 40 commits into
pydata:masterfrom
jukent:fixtests
Sep 9, 2020
Merged

Preserve attrs with coarsen#4360
dcherian merged 40 commits into
pydata:masterfrom
jukent:fixtests

Conversation

@jukent
Copy link
Copy Markdown
Contributor

@jukent jukent commented Aug 20, 2020 •
edited by dcherian
Loading

@jukent
pass **kwargs into
991b58b 
_coarsen_reshape
@jukent jukent changed the title pass **kwargs into pass **kwargs into _coarsen_reshape Aug 20, 2020
@jukent jukent changed the title pass **kwargs into _coarsen_reshape _coarsen_reshape Aug 20, 2020
@jukent jukent changed the title _coarsen_reshape fixes to variable.py/coarsen Aug 20, 2020
@jukent
Copy link
Copy Markdown
Contributor Author

jukent commented Aug 20, 2020

This PR passes **kwargs into _coarsen_reshape and changes _replace to copy in the fx return of coarsen.

@jukent jukent mentioned this pull request Aug 20, 2020
Closed
@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Aug 27, 2020 •
edited
Loading

Hello @jukent! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-09-09 02:25:19 UTC

@jukent jukent marked this pull request as ready for review September 3, 2020 21:08
dcherian
dcherian reviewed Sep 3, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@dcherian dcherian 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 I have some minor suggestions to make the code clearer

Comment thread doc/whats-new.rst Outdated
Comment thread xarray/core/variable.py
Comment thread xarray/core/variable.py
Comment thread xarray/tests/test_dataset.py
@dcherian dcherian changed the title fixes to variable.py/coarsen Preserve attrs with coarsen Sep 3, 2020
dcherian
dcherian reviewed Sep 3, 2020
View reviewed changes
Comment thread .pre-commit-config.yaml
jukent and others added 2 commits September 8, 2020 09:56
@jukent @dcherian
Update doc/whats-new.rst
f5c247a 
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
@jukent
go back to v0.1.2 of blackdock
f0cf2ef
max-sixty
max-sixty reviewed Sep 8, 2020
View reviewed changes
Comment thread xarray/tests/test_dataarray.py
dcherian
dcherian reviewed Sep 8, 2020
View reviewed changes
Comment thread xarray/core/variable.py Outdated
@max-sixty
Copy link
Copy Markdown
Collaborator

max-sixty commented Sep 9, 2020

@jukent looks like black may be causing a couple of issues. I'm pushing a fix. You might want to run python3 -m pip install --upgrade black to get the latest version of black — it recently changed version

@max-sixty
Copy link
Copy Markdown
Collaborator

max-sixty commented Sep 9, 2020

(you should git pull from your branch, which is now updated with the fix I pushed — let me know any issues)

dcherian
dcherian reviewed Sep 9, 2020
View reviewed changes
Comment thread xarray/core/variable.py Outdated
@jukent @dcherian
Update xarray/core/variable.py
438de49 
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
@dcherian dcherian added the topic-metadata Relating to the handling of metadata (i.e. attrs and encoding) label Sep 9, 2020
@jukent jukent requested a review from dcherian September 9, 2020 17:29
max-sixty
max-sixty approved these changes Sep 9, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

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

Super! This looks great, thanks @jukent .

I'll let @dcherian hit the green button assuming he's fine with it

@dcherian
Copy link
Copy Markdown
Contributor

dcherian commented Sep 9, 2020

Thanks @jukent This is a nice fix to a complicated part of the code base!

@dcherian dcherian merged commit 572a528 into pydata:master Sep 9, 2020
@kmpaul
Copy link
Copy Markdown
Contributor

kmpaul commented Sep 9, 2020

Nice work, @jukent!

@jukent jukent mentioned this pull request Sep 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@keewis keewis keewis left review comments

@max-sixty max-sixty max-sixty approved these changes

@dcherian dcherian Awaiting requested review from dcherian

Assignees

No one assigned

Labels

topic-metadata Relating to the handling of metadata (i.e. attrs and encoding)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

coarsen deletes attrs on original object

6 participants

@jukent @pep8speaks @max-sixty @dcherian @kmpaul @keewis