Add EnsembleFrame Support to Tape #308

wilsonbb · 2023-12-04T19:38:38Z

Refactors TAPE to add support for tape.EnsembleFrames, a TAPE-specific extension of Dask DataFrames

While the Ensemble-level API for manipulating the Source and Object frames is still supported, users now

An EnsembleFrame is a Dask DataFrame tracked by the Ensemble which has

Direct support for using the Dask API on the underlying frame
tracking for whether or not it is a 'dirty' table due to an update rows and whether the table may need to be synced
an associated identifying label
a reference back to the Ensemble object which tracks the frame. This allows the frame to inform the Ensemble when a sync between tables (currently only Object and Source) may be required.

The Ensemble now can track multiple EnsembleFrames

Supporting functions such as Ensemble.select_frame(label), Ensemble.update_frame(frame), Ensemble.drop_frame(frame)
Access to the Object and Source frames as Ensemble.object and Ensemble.source which are subclasses of `Ensemble
Calls to the Object and Source tables such as result = Ensemble.source.query() now return a view of the updated frame without affecting the underlying Source and Object tables, until result.update_ensemble() is called
Minimal wrapper subclasses of Pandas dataframes such as TapeFrame, TapeObjectFrame, and TapeSourceFrame are used as Dask meta to help Dask infer which EnsembleFrame object to instantiate
A unique label is generated for tracking EnsembleFrames which are the output of Ensemble.batch. Users can specify their own label as an argument.

Note that the commits in this PR were independently reviewed as they were added to the tape_ensemble_refactor branch. (One exception is setting client=False in test_analysis.py to reduce flakiness from reusing a disconnected client).

…working A minimal Dask Dataframe subclass for the Ensemble

…working Make convert_flux_to_mag part of the EnsembleFrame

…working Ensembles can now track a group of labeled frames

…working Preserve EnsembleFrame metadata after assign()

Merge main into tape_ensemble_refactor

Merge updates from main into refactor working branch

…working Subclass EnsembleFrames for Object and Source Tables

* Adds EnsembleFrame.update_ensemble() * Use EnsembleFrames throughout the Ensemble * Udpdate ensemble test * Extends update_ensemble test cases * Unpin sphinx to address docs build fail * Fix minor test error * Remove debug line

* EnsembleFrames should propagate is_dirty * Test that a frame's dirty status propagates * Update doc strings * Address review comment

* Add ensemble loader functions for dataframes * Updated unit tests * Lint fixes * Always update column mapping * Addressed review comments * Ensure object frame is indexed * adds a dask_on_ray tutorial * add performance comp; add use_map comment --------- Co-authored-by: Doug Branton <[email protected]>

@staticmethod

* Add ensemble loader functions for dataframes * Updated unit tests * Lint fixes * Always update column mapping * Addressed review comments * Ensure object frame is indexed * adds a dask_on_ray tutorial * add performance comp; add use_map comment * sync with map_partitions * sync with map_partitions * sync with map_partitions * sync with map_partitions * coalesce with map_partitions * use dataframes instead of series * add descriptive comments * implement suggestions * Update TAPE README.md Update the project description for TAPE to better reflect the current state and goals of the project. * Set object table index for from_dask_dataframe * add zero_point as float input :q q * add ensemble default cols * S82 RRLyr notebook * Move rrlyr nb to examples * Update requirements.txt to unpin sphinx * Update pyproject.toml to unpin sphinx * add calc_nobs * add calc_nobs * add calc_nobs * reduce scope of sync_tables * address divisions issue * add temporary cols test * improve coverage * add temporary kwarg to assign * add temporary kwarg to assign * drop divisions * drop brackets * fix bug in sync * Issue 199: Added static Ensemble read constructors to tape namespace (#256) * Added static read constructors to tape namespace * Removed @staticmethod as python 3.9 didn't like it * Reformatted via black * Changed read_dask_dataframe to call from_ method * Collapsed create dask client args to single arg * Fixed dask_client parameter * reformatted via black * Added missing unit test * Resolved code review comments from PR 256 * Fixed failing unit test Removed reference to Ensemble._nobs_band_cols field * fix bug in sync --------- Co-authored-by: Doug Branton <[email protected]> Co-authored-by: Konstantin Malanchev <[email protected]> Co-authored-by: Olivia R. Lynn <[email protected]> Co-authored-by: Chris Wenneman <[email protected]>

…280) * FIx _Frame.set_dirty * Update propgating data in map_partitions * Fix typo

…g if row count changed (#281) * Mark frames dirty without len() call * Move calls to set_dirty to EnsembleFrame

Merge main into tape_ensemble_refactor

* Add meta handling for batch * Add unit tests for custom meta * Remove unit test sanity check, fix warning output * Provide default labels for result frames.

* check divisions, enable lazy syncs * check divisions, enable lazy syncs * initial tests * add tests; calc_nobs preserve divisions * batch with divisions * cleanup * fix sf2 tests * add sync_tables check * cleanup * fix calc_nobs reset_index issue * per table warnings; index comments * add map_partitions mode for calc_nobs when divisions are known * build metadata * build metadata * add multi partition test * add version file to init * add small test * Fix table syncing to use inner joins. (#303) * Fix table syncing to use inner joins. * fix lint error * Update test --------- Co-authored-by: Doug Branton <[email protected]>

This reverts commit 5c847e1.

…_the_merge Revert "Merge Main into Ensemble Refactor Branch"

Merge Main into Ensemble Refactor Branch

review-notebook-app · 2023-12-04T19:38:43Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

codecov · 2023-12-05T00:10:36Z

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (626904c) 94.03% compared to head (a415078) 94.46%.

Files	Patch %	Lines
src/tape/ensemble.py	91.19%	17 Missing ⚠️
src/tape/ensemble_frame.py	96.28%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #308      +/-   ##
==========================================
+ Coverage   94.03%   94.46%   +0.43%     
==========================================
  Files          23       24       +1     
  Lines        1207     1518     +311     
==========================================
+ Hits         1135     1434     +299     
- Misses         72       84      +12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/tape/ensemble.py

hombit

LGTM! I have few minor comments, but we could convert them to issues to not block the PR. Thank you, @wilsonbb and @dougbrn!

src/tape/__init__.py

src/tape/ensemble.py

src/tape/ensemble_frame.py

hombit · 2023-12-05T16:28:12Z

src/tape/ensemble_frame.py

+        new_frame.set_dirty(self.is_dirty())
+        return new_frame
+
+    def copy(self):


Do we need to implement __copy__()?

I feel that logically we should, but neither Dask in its overrides of Pandas nor dask-geopandas did so. I was a bit surprised by this and forgot about it, but I should revisit.

src/tape/ensemble_frame.py

dougbrn

This looks good, one question on whether to remove some Ensemble functions, but this can just be logged in an issue if we end up wanting to do that. Thanks so much for all the great work on this @wilsonbb !

src/tape/ensemble.py

wilsonbb · 2023-12-08T01:41:06Z

LGTM! I have few minor comments, but we could convert them to issues to not block the PR. Thank you, @wilsonbb and @dougbrn!

Thank you for the time taken to look at this!

wilsonbb and others added 30 commits August 25, 2023 15:38

A minimal Dask Dataframe subclass for the Ensemble

f8c420b

Addressed comments, added test fixture.

740d2d7

Merge pull request #209 from lincc-frameworks/tape_ensemble_refactor_…

d033607

…working A minimal Dask Dataframe subclass for the Ensemble

Make convert_flux_to_mag part of the EnsembleFrame

9a61392

Merge pull request #212 from lincc-frameworks/tape_ensemble_refactor_…

17f9cc1

…working Make convert_flux_to_mag part of the EnsembleFrame

Ensembles can now track a group of labeled frames

72b8629

Merge pull request #217 from lincc-frameworks/tape_ensemble_refactor_…

1cd049e

…working Ensembles can now track a group of labeled frames

Preserve EnsembleFrame metadata after assign()

3128141

Merge pull request #218 from lincc-frameworks/tape_ensemble_refactor_…

01a083a

…working Preserve EnsembleFrame metadata after assign()

Merge pull request #219 from lincc-frameworks/main

44ef76e

Merge main into tape_ensemble_refactor

Merge pull request #220 from lincc-frameworks/tape_ensemble_refactor

423805a

Merge updates from main into refactor working branch

Parquet support for frame subclasses checkpoint

8db79e0

Reverting changes to tests

657a2a7

Adds test for objsor_from_parquet

e8de263

Addressed comments

34e9bbd

Removed adding column via apply

93abf4d

Fix comment typo

8c8e793

Merge pull request #222 from lincc-frameworks/tape_ensemble_refactor_…

c586fa1

…working Subclass EnsembleFrames for Object and Source Tables

Fix EnsembleFrame.set_index

068870a

Add update_ensemble() and Use EnsembleFrames (#252)

db8a1ab

* Adds EnsembleFrame.update_ensemble() * Use EnsembleFrames throughout the Ensemble * Udpdate ensemble test * Extends update_ensemble test cases * Unpin sphinx to address docs build fail * Fix minor test error * Remove debug line

Propagate EnsembleFrame._is_dirty (#264)

13d507b

* EnsembleFrames should propagate is_dirty * Test that a frame's dirty status propagates * Update doc strings * Address review comment

Have update_frame mark frames as dirty (#267)

578900f

Remove calls to set_dirty in ensemble (#269)

35de81c

Fix EnsembleFrame.set_dirty and map_partitions metadata propagation (#…

0d4da10

…280) * FIx _Frame.set_dirty * Update propgating data in map_partitions * Fix typo

Ensemble.update_frame no longer infers if a frame is dirty by checkin…

c86d7ab

…g if row count changed (#281) * Mark frames dirty without len() call * Move calls to set_dirty to EnsembleFrame

Merge branch 'main' into big_merge

a980349

Merge pull request #282 from lincc-frameworks/update_merge

4b7d01e

Merge main into tape_ensemble_refactor

Support storing batch results for custom meta (#285)

0ce6f2e

* Add meta handling for batch * Add unit tests for custom meta * Remove unit test sanity check, fix warning output * Provide default labels for result frames.

wilsonbb and others added 7 commits November 30, 2023 16:32

Update test

8f8cc66

Merge branch 'main' into night_before_the_merge

1b150eb

Revert "Merge Main into Ensemble Refactor Branch (#304)"

6779ba0

This reverts commit 5c847e1.

Merge pull request #306 from lincc-frameworks/revert-304-night_before…

8154ee9

…_the_merge Revert "Merge Main into Ensemble Refactor Branch"

Merge pull request #307 from lincc-frameworks/night_before_the_merge

3c4a484

Merge Main into Ensemble Refactor Branch

Fix linting

494eefc

Remove unsupported type annotations

046942b

This was linked to issues Dec 4, 2023

EnsembleFrame Refactor #191

Closed

EnsembleFrame Refactor Milestone 3: The ObjectFrame and SourceFrame #194

Closed

EnsembleFrame Refactor Milestone 4: Refactor Ensemble logic #195

Closed

wilsonbb added 2 commits December 4, 2023 14:10

Fix merge error

dd22e9e

Use client=False in test_analysis

3de618f

wilsonbb marked this pull request as ready for review December 5, 2023 00:20

wilsonbb requested review from dougbrn and hombit December 5, 2023 00:20

dougbrn reviewed Dec 5, 2023

View reviewed changes

src/tape/ensemble.py Show resolved Hide resolved

dougbrn reviewed Dec 5, 2023

View reviewed changes

src/tape/ensemble.py Show resolved Hide resolved

wilsonbb added 2 commits December 5, 2023 15:42

Remove '_object' and '_source' fields

d061b3c

Fix linting errors

81bd28c

wilsonbb requested a review from dougbrn December 5, 2023 23:51

hombit approved these changes Dec 6, 2023

View reviewed changes

dougbrn approved these changes Dec 7, 2023

View reviewed changes

src/tape/ensemble.py Outdated Show resolved Hide resolved

src/tape/ensemble.py Show resolved Hide resolved

Address review comments, add tests

a415078

wilsonbb merged commit 1e8abff into main Dec 8, 2023
10 checks passed

wilsonbb mentioned this pull request Dec 18, 2023

Reduce TAPE Unit Test Flakiness #328

Closed

dougbrn deleted the final_merge branch April 4, 2024 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add EnsembleFrame Support to Tape #308

Add EnsembleFrame Support to Tape #308

wilsonbb commented Dec 4, 2023 •

edited

Loading

review-notebook-app bot commented Dec 4, 2023

codecov bot commented Dec 5, 2023 •

edited

Loading

hombit left a comment

hombit Dec 5, 2023

wilsonbb Dec 8, 2023

dougbrn left a comment

wilsonbb commented Dec 8, 2023

Add EnsembleFrame Support to Tape #308

Add EnsembleFrame Support to Tape #308

Conversation

wilsonbb commented Dec 4, 2023 • edited Loading

review-notebook-app bot commented Dec 4, 2023

codecov bot commented Dec 5, 2023 • edited Loading

Codecov Report

hombit left a comment

Choose a reason for hiding this comment

hombit Dec 5, 2023

Choose a reason for hiding this comment

wilsonbb Dec 8, 2023

Choose a reason for hiding this comment

dougbrn left a comment

Choose a reason for hiding this comment

wilsonbb commented Dec 8, 2023

wilsonbb commented Dec 4, 2023 •

edited

Loading

codecov bot commented Dec 5, 2023 •

edited

Loading