Chardata covcheck#7137
Conversation
![scitools-ci[bot]](https://avatars.githubusercontent.com/in/212324?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
pp-mo
dismissed
scitools-ci[bot]’s stale review
Change exists on parent branch, will no longer appear here when that is merged
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## FEATURE_chardata #7137 +/- ##
====================================================
+ Coverage 90.24% 90.37% +0.13%
====================================================
Files 92 92
Lines 25151 25775 +624
Branches 4698 4901 +203
====================================================
+ Hits 22697 23294 +597
- Misses 1682 1691 +9
- Partials 772 790 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # Turn off *any* automatic decoding in the underlying netCDF4 dataset | ||
| ds = self._dataset | ||
| if isinstance(ds, _thread_safe_nc.DatasetWrapper): | ||
| ds._contained_instance.set_auto_chartostring(False) | ||
| else: | ||
| ds.set_auto_chartostring(False) |
There was a problem hiding this comment.
I know that this was already merged in #7101 , but I've just realised: wouldn't it be better to implement set_auto_chartostring within _thread_safe_nc?
There was a problem hiding this comment.
The idea was that no downstream code would ever need awareness of the kind of object it was handling - the wrapper objects should behave precisely identical.
There was a problem hiding this comment.
Okay I'm now checking this out -- new code testing in CI...
Looking again, I think it might be worth making tests/units...test_bytecoding_datasets a bit more comprehensive.
Maybe don't merge until I've thought about this a bit more.
pp-mo
force-pushed
the
chardata_covcheck
branch
2 times, most recently
from
f913e70 to
5ca639f
Compare
2 participants
Closes #6921
Addressing coverage holes noted there
Incomplete at present
Based on code for #7101, will rebase when that is in.