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

Fix keep_yboundaries=False for squashed, double-null cases #180

Merged
merged 7 commits into from
Mar 19, 2021

Conversation

johnomotani
Copy link
Collaborator

@johnomotani johnomotani commented Jan 15, 2021

Previously squashed Datasets (which include boundary cells*) representing double-null cases with a second boundary could only be opened with keep_yboundaries=True, because when the single file is loaded the boundary cells from the second boundary are always present, and since they are not at the edge like guard cells, they are not easy to remove in the _trim() function. Fix this case by detecting squashed, double-null Datasets, loading them with keep_yboundaries=True, and then calling ds.bout.remove_yboundaries() afterwards.

* squashed Datasets that do not include y-boundary cells were fine, because in that case keep_yboundaries has nothing to do anyway.

Required some small changes so that remove_yboundaries() could always be called - previously if no geometry was set some metadata and attrs["geometry"] were not set, but are used in remove_yboundaries().

Updates to the test-data creation routines to improve the creation of 'squashed' Datasets to test the new feature.

Fixes #113.
Fixes #176.

@codecov-io
Copy link

codecov-io commented Jan 15, 2021

Codecov Report

Merging #180 (99b4480) into master (f54e8f8) will increase coverage by 0.07%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
+ Coverage   75.81%   75.88%   +0.07%     
==========================================
  Files          14       14              
  Lines        2394     2418      +24     
  Branches      553      556       +3     
==========================================
+ Hits         1815     1835      +20     
- Misses        381      385       +4     
  Partials      198      198              
Impacted Files Coverage Δ
xbout/load.py 78.57% <80.95%> (-0.77%) ⬇️
xbout/boutdataset.py 75.60% <100.00%> (+0.52%) ⬆️
xbout/geometries.py 79.19% <100.00%> (+0.72%) ⬆️
xbout/utils.py 81.96% <100.00%> (+0.23%) ⬆️

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 f54e8f8...99b4480. Read the comment docs.

xbout/load.py Outdated Show resolved Hide resolved
Part of code comment related to an implementation detail that was reverted.
@pep8speaks
Copy link

pep8speaks commented Mar 19, 2021

Hello @johnomotani! 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 2021-03-19 14:29:32 UTC

xbout/load.py Outdated Show resolved Hide resolved
@johnomotani johnomotani merged commit 0e58b2c into master Mar 19, 2021
@johnomotani johnomotani deleted the fix-doublenull-keep_yboundaries branch March 19, 2021 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix for a bug
Projects
None yet
3 participants