Add top-mass and bulk-reference metrics for traps; make remove_traps accept traps DataFrame and return verification#360
Open
charlesmartin14 wants to merge 1 commit intomasterfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
remove_trapsAPI to accept the oldertrapsDataFrame shape for compatibility and to support post-removal verification and optional returned analysis.Description
_top_percent_abs_massand integratedtop_5_massandtop_10_massper-trap, and addedcompute_bulk_trap_reference_metricsto computebulk_mode_count,bulk_localization_mean,bulk_localization_std,bulk_top_5_mass_mean,bulk_top_5_mass_std,bulk_top_10_mass_mean, andbulk_top_10_mass_stdreturned with each trap row and included in theWeightWatcherresult columns.remove_trapsflow with_trap_indices_from_traps_dfto accept atrapsDataFrame-like argument (backwards-compatible PR359 path) and to normalize indices; addedverify_trapsandreturn_analyzeoptions soremove_trapscan perform post-removal artifact collection and return a verificationDataFramewhen requested.WeightWatcherwrapper signatures to accepttraps,verify_traps, andreturn_analyze, and delegated behavior to the newremove_trapsimplementation inremove_traps.py.pandasimport where needed and updated expected trap-analysis columns intests/test_analyze_traps.pyand addedtests/test_remove_traps.py::test_remove_traps_accepts_traps_dataframe_and_returns_verify_dfto exercise the DataFrame/verify path.Testing
tests/test_analyze_traps.pywhich assert new trap columns are present and reproducibility behavior remains unchanged, and these tests passed.tests/test_remove_traps.py, includingtest_remove_traps_accepts_traps_dataframe_and_returns_verify_df, which passed and verifies the DataFrame input and returned verification frame behavior.remove_traps/analyze_trapscode paths used by the new tests to confirm end-to-end behavior in the updated test suite.Codex Task