Partial collapse of multi-dimensional string coords #4294
Conversation
| bounds = [] | ||
| for index in np.ndindex(shape): | ||
| index_slice = (slice(None),) + tuple(index) | ||
| bounds.append(serialize(self.bounds[index_slice])) |
There was a problem hiding this comment.
If I've understood the old slicing method, it and the new method are consistent for the basic case of 2D bounds (n_points, n_bounds). For higher dimensions, the old method doesn't look right to me: if we had a 2D (n, m) coordinate with bounds (n, m, n_bounds), it looks like we would get m * n_bounds elements in the list. But I think we would have wanted just n_bounds elements.
I didn't even know bounded string coords were a thing!
lib/iris/coords.py
Outdated
| # Create the new collapsed coordinate. | ||
| coord = self.copy( | ||
| points=np.array(points, dtype=dtype), bounds=bounds |
There was a problem hiding this comment.
I don't know why it was necessary to set the dtype, but the existing tests pass fine without this now. The old code is 8 years old, so I guess numpy has moved on and is likely doing more for us automatically.
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
484a02b
to
9a87565
Compare
|
Rebased. CI is green. Ready for review 😎 |
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
9a87565
to
13a5a12
Compare
|
Added some type hints for the method and function I modified, as well as ones required to tidy up PyRight complaints within the |
|
now redundant as we turned hints off in the docs at #4510
|
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
13a5a12
to
29e312f
Compare
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
0361de4
to
af84640
Compare
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
af84640
to
a8186c7
Compare
|
@bjlittle get your finger out... |
|
@rcomer Fancy doing a rebase? |
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
a8186c7
to
668251d
Compare
|
Now rebased. @bjlittle it's all on you now 😛 |
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
668251d
to
8820c40
Compare
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
8820c40
to
1eebe2a
Compare
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
1eebe2a
to
4a3c441
Compare
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
4a3c441
to
68082f0
Compare
rcomer
force-pushed
the
collapse-2d-string-coord
branch
from
cb44650
to
dd13194
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4294 +/- ##
==========================================
+ Coverage 89.76% 89.78% +0.02%
==========================================
Files 93 93
Lines 22982 23015 +33
Branches 5006 5021 +15
==========================================
+ Hits 20630 20665 +35
+ Misses 1622 1620 -2
Partials 730 730 ☔ View full report in Codecov by Sentry. |
|
See #5955 for a version of this fix without the type-hinting rabbit hole. |
🚀 Pull Request
Description
Fixes #3653.
If you have a 2D string coordinate on a cube, and collapse one of the dimensions spanned by that coordinate, the coordinate's points (and bounds, if any) are now serialised along the collapsed dimension. This seems to have been missed when partial collapse was implemented for the numeric coordinates.
Consult Iris pull request check list