DM-39939: use query-results' grouping when processing iterables of DatasetRefs #863
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #863 +/- ##
==========================================
+ Coverage 87.90% 87.92% +0.01%
==========================================
Files 273 270 -3
Lines 35764 35697 -67
Branches 7474 7478 +4
==========================================
- Hits 31440 31388 -52
+ Misses 3166 3150 -16
- Partials 1158 1159 +1
☔ View full report in Codecov by Sentry. |
TallJimbo
force-pushed
the
tickets/DM-39939
branch
2 times, most recently
from
45e7f53
to
8213990
Compare
| if hasattr(refs, "_iter_by_dataset_type"): | ||
| return refs._iter_by_dataset_type() |
There was a problem hiding this comment.
I'm not super-happy with this dynamic approach, particularly because our favorite mypy cannot type-check this. Would it be possible to add an abstraction (or maybe Protocol with runtime check) so that isinstance can be used instead?
There was a problem hiding this comment.
I've switched to a runtime-checkable protocol, but I've given it a leading underscore (and documented it as "package private") since I don't want external code to start using this.
| refs : `~collections.abc.Iterable` [ `DatasetRef` ] | ||
| `DatasetRef` instances to group. If this has a | ||
| ``_iter_by_dataset_type`` method, it will be called with no | ||
| arguments and the result reutrnd. |
There was a problem hiding this comment.
| arguments and the result reutrnd. | |
| arguments and the result returned. |
tests/test_progress.py
Outdated
| self.assertEqual(MockProgressBar.last.total, 2) | ||
|
|
||
| def test_iter_item_chunks_not_sized(self): | ||
| """Test using `Progress.iter_item_chunks` with an unsized iterable |
There was a problem hiding this comment.
| """Test using `Progress.iter_item_chunks` with an unsized iterable | |
| """Test using `Progress.iter_item_chunks` with an unsized iterable of |
TallJimbo
force-pushed
the
tickets/DM-39939
branch
from
8213990
to
8a9e8c4
Compare
When processing all dataset types in a collection together, this can represent a huge decrease in memory usage, by querying for and then processing only one dataset type at a time.
TallJimbo
force-pushed
the
tickets/DM-39939
branch
from
8a9e8c4
to
a3ec38a
Compare
Checklist
doc/changes