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

[asdf/xarray.DataSet|DataArray] fix serialization of attributes #384

Conversation

marscher
Copy link
Contributor

@marscher marscher commented Jun 29, 2021

Changes

The wrapping of Variables inside the DataSet serialization did not contain the attrs property. Additionally, the coordinates
of a DataArray can also enclose attributes, these were also not being serialized.

Refactored some for loops in favor of list comprehensions.

Related Issues

Closes #349

Checks

  • updated CHANGELOG.md
  • updated tests
  • updated doc/
  • update example/tutorial notebooks

@codecov
Copy link

codecov bot commented Jun 29, 2021

Codecov Report

Merging #384 (5c8a07e) into master (1999c8d) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #384      +/-   ##
==========================================
- Coverage   97.44%   97.43%   -0.01%     
==========================================
  Files          86       86              
  Lines        5196     5183      -13     
==========================================
- Hits         5063     5050      -13     
  Misses        133      133              
Impacted Files Coverage Δ
weldx/asdf/tags/weldx/core/common_types.py 100.00% <100.00%> (ø)
weldx/asdf/tags/weldx/core/data_array.py 100.00% <100.00%> (ø)
weldx/asdf/tags/weldx/core/dataset.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1999c8d...5c8a07e. Read the comment docs.

@marscher
Copy link
Contributor Author

Funnily the asdf tests pass on Python-3.9, so I guess the dataclassses.field(defaultfactory=dict) thing behaves differently on 3.8...

@marscher
Copy link
Contributor Author

Ah this is the schema tests failing. Sry. So do I need to adopt the schema as well, indicating the new optional "attrs" field should be there?
Please enlighten me @vhirtham, @CagtayFabry

@BAMWelDX BAMWelDX deleted a comment from pep8speaks Jun 29, 2021
@marscher marscher requested review from CagtayFabry and vhirtham and removed request for CagtayFabry June 30, 2021 07:44
@marscher marscher changed the title [asdf/xarray.DataSet] fix serialization of child attrs. [asdf/xarray.DataSet|DataArray] fix serialization of attributes Jun 30, 2021
Copy link
Collaborator

@vhirtham vhirtham left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanups. Really funny to see all the bad lines of code that I wrote when I started with Python 😄

weldx/asdf/tags/weldx/core/common_types.py Outdated Show resolved Hide resolved
weldx/asdf/tags/weldx/core/data_array.py Show resolved Hide resolved
weldx/asdf/tags/weldx/core/dataset.py Outdated Show resolved Hide resolved
weldx/asdf/tags/weldx/core/dataset.py Outdated Show resolved Hide resolved
Co-authored-by: vhirtham <volker.hirthammer@gmail.com>
@marscher marscher merged commit 525b5c0 into BAMWelDX:master Jul 1, 2021
@marscher marscher deleted the fix_data_array_attrs_from_dataset_serialization branch July 1, 2021 18:14
@CagtayFabry CagtayFabry added ASDF everything ASDF related (python + schemas) core weldx core classes and functions labels Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASDF everything ASDF related (python + schemas) core weldx core classes and functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataArray attributes missing from Dataset ASDF serialization
3 participants