Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2912 +/- ##
=======================================
Coverage 83.37% 83.38%
=======================================
Files 248 248
Lines 51662 51685 +23
Branches 4459 4460 +1
=======================================
+ Hits 43073 43096 +23
Misses 7834 7834
Partials 755 755
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mloubout
force-pushed
the
misc-sparse-fix
branch
3 times, most recently
from
775e909 to
6367e85
Compare
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| return self.grid.dimensions | ||
|
|
||
| @cached_property | ||
| @property |
There was a problem hiding this comment.
bad sign -- is it symptomatic of a change of state?
There was a problem hiding this comment.
self.sfunction._crdim is memoized, caching on top doesn't have any benefit and would actually double cache it
There was a problem hiding this comment.
that's in a different class and we shouldn't generally rely on that, so I'd keep the cached_property
| return self._dist_origin | ||
|
|
||
| @memoized_meth | ||
| def _crdim(self, dim): |
There was a problem hiding this comment.
unclear what a "crdim" is, so I think a docstring is due
There was a problem hiding this comment.
"custom radius dimension" but sure can add docstring
| self.r, 2*self.r, self._sparse_dim) | ||
|
|
||
| @memoized_meth | ||
| def _cond_rdim(self, dim, cond): |
There was a problem hiding this comment.
I also struggle with this name
| return self.grid.dimensions | ||
|
|
||
| @cached_property | ||
| @property |
There was a problem hiding this comment.
that's in a different class and we shouldn't generally rely on that, so I'd keep the cached_property
| def _crdim(self, dim): | ||
| """ | ||
| The CustomDimension associated with the Dimension `dim` for | ||
| the radius of the interpolation/injection stencil |
There was a problem hiding this comment.
ultra-nitpick: full stop
| @memoized_meth | ||
| def _cond_rdim(self, dim, cond): | ||
| """ | ||
| The interpolation/injection radius dimension with guard bounds |
There was a problem hiding this comment.
ultra-nitpick: full stop
There was a problem hiding this comment.
maybe also "guarded"?
2 participants
See
https://devitocodes.slack.com/archives/CVC6JFZ4N/p1777774975142759?thread_ts=1777774396.183819&cid=CVC6JFZ4N