fix(cellbudgetfile): fix get_ts support for aux vars #2648
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2648 +/- ##
===========================================
+ Coverage 55.5% 72.7% +17.1%
===========================================
Files 644 667 +23
Lines 124135 129516 +5381
===========================================
+ Hits 68947 94162 +25215
+ Misses 55188 35354 -19834
🚀 New features to boost your workflow:
|
| """ | ||
| out = np.ma.zeros(self.nnodes, dtype=np.float32) | ||
| out = np.ma.zeros(self.nnodes, dtype=data["q"].dtype) |
There was a problem hiding this comment.
unrelated fix to avoid unwanted rounding in the full3D code path
| spd = getattr(self, "stress_period_data") | ||
| if isinstance(item, MfList): | ||
| if not isinstance(item, list) and not isinstance(item, tuple): | ||
| if not isinstance(item, (list, tuple)): |
There was a problem hiding this comment.
unrelated opportunistic simplification
| [(layer, row, column), (layer, row, column), ...]. The layer, | ||
| row, and column values must be zero based. | ||
| idx : int, tuple of ints, or list of such | ||
| Acceptable values depend on grid type: |
There was a problem hiding this comment.
Described required syntax for each grid type as requested. But I tentatively reworked this to accept a properly-shaped tuple for the grid type. Having to pass a 3-tuple with dummy values was very awkward and I'm tempted to call this a fix even though it's a breaking change. I don't know of anywhere else this is done that way. Wondering how others feel about this.
There was a problem hiding this comment.
idk why I was thinking we had to pick one or the other way, it's unambiguous with the 3-tuple since you already know what type the grid is. doh. added support for the old way so this is no longer breaking.
|
@cneyens do you mind having a look at this before I merge? |
@wpbonelli thanks for this! I've checked with the example in #2647 which now works with both DIS and DISV grids. For DISV grids, the same results are returned for qx when using the original three-indices cell id (with dummy value in the second position) and the new two-indices approach. Can this also be implemented for the head object, so Time series of other fluxes are also working as expected, except STO-SS with a DISV grid (regardless of using three-indices or two-indices for the idx argument): flopy/flopy/utils/binaryfile/__init__.py Line 1724 in 1336e60 returning error |
|
Thanks @cneyens nice catch. I guess the fact that
|
2 participants
close #2647, correcting initial buggy implementation of #2270