Conversation
* stepwise modelling * Update tfbpmodeling/loop_modeling.py Co-authored-by: Chase Mateusiak <chasem@wustl.edu> * Update tfbpmodeling/loop_modeling.py Co-authored-by: Chase Mateusiak <chasem@wustl.edu> * Update tfbpmodeling/loop_modeling.py Co-authored-by: Chase Mateusiak <chasem@wustl.edu> * with pre-commit * removing islice from bootstrap loop * removing fixtures to conftest; fixing spacing in loop module; adding pytest to CI * Set random state on bootstraps; Remove unweighted bootstrap option (#31) * intermediate * propogating bootstrappedmodelinginputdata changes to __main__ * removed unweighted bootstrap options * removing top_n as argparse option from step3 sigmoid parser * adding center_scale to argparse * setting drop_intercept to True permanently for sigmoid worker * the sigmoid parameters must have args.drop_intercept still * handling intercepts * fixing typo in center_scale logging * changing the way the formula is logged * removing truncation from formula logging * adding logging on random_state in bootstrappedmodelinput * removing sample weight cv log * removing sample weight cv logging * loop exits if no variable selected within the loop, fixes issue 34 (#37) * loop exits if no variable selected within the loop, fixes issue 34 * fixing linter issues * linter issues --------- Co-authored-by: Zolboo Erdenebaatar <e.zolboo@login.adm> Co-authored-by: zolboo e <admin@zolboos-MacBook-Pro.local> * adding feature stage4_topn (#43) * adding feature stage4_topn * modifying argument parser * Align response_df with predictors from get_modeling_data to ensure consistency when top_n_masked is enabled * Align response_df with predictors from get_modeling_data to ensure consistency when top_n_masked is enabled * aligning stratified_cv_r2 * restoring changes to evaluate_interactor_significance_linear * commit after fixing error in stratification_classification * attempt to fixing inconsistent numbers of samples * fixing pr --------- Co-authored-by: chasem <chasem@wustl.edu> * Remove bin by binding and Add check on number of features and increase minimal test case size (#54) * saving changes for remove_bin_by_binding * Add check on number of features and increase minimal test case size * Separate the functions and objects in lasso_modeling.py (#58) * create interface.py to separate main * add tests to verify important logging statements * move the logging configuration back into main * create interface.py to separate main * add tests to verify important logging statements * move the logging configuration back into main * fixing interface * removing bin_by_binding_only from test arguments to interface * adding name == main to main script * create interface.py to separate main * move the logging configuration back into main * removing bin_by_binding_only from test arguments to interface * rebasing refactor onto dev * adding calling to main * adding a feature column in test_interface * separate the functions and objects in lasso_modeling.py * Refactor main by adding interface.py (#56) * create interface.py to separate main * add tests to verify important logging statements * move the logging configuration back into main * create interface.py to separate main * add tests to verify important logging statements * move the logging configuration back into main * fixing interface * removing bin_by_binding_only from test arguments to interface * adding name == main to main script * create interface.py to separate main * move the logging configuration back into main * removing bin_by_binding_only from test arguments to interface * rebasing refactor onto dev * adding calling to main * adding a feature column in test_interface --------- Co-authored-by: chasem <chasem@wustl.edu> * create interface.py to separate main move the logging configuration back into main fixing interface create interface.py to separate main move the logging configuration back into main removing bin_by_binding_only from test arguments to interface rebasing refactor onto dev create interface.py to separate main move the logging configuration back into main removing bin_by_binding_only from test arguments to interface adding name == main to main script separate the functions and objects in lasso_modeling.py * renaming loop_modeling * separate the tests out into files * separate the tests out into files * fixing evaluate_interactor_significance_lassocv --------- Co-authored-by: chasem <chasem@wustl.edu> * removing a rebase error * renaming exclude_interaction_variable to exclude_model_variable * clarifying cmd line instructions * updating macos github actions version * Update README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fixing plural on exclude_model_variables * Update tfbpmodeling/interface.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update tfbpmodeling/interface.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fixing some documentation issues with plural/singular and names around exclude and include variables * Update docs/cli/linear-perturbation-binding-modeling.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Zolboo Erdenebaatar <e.zolboo@n039.adm> Co-authored-by: ezolbooe <e.zolboo@wustl.edu> Co-authored-by: zolboo e <admin@zolboos-macbook-pro.local.dhcp.wustl.edu> Co-authored-by: Zolboo Erdenebaatar <e.zolboo@login.adm> Co-authored-by: zolboo e <admin@zolboos-MacBook-Pro.local> Co-authored-by: 17TML <liuchenxing9@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #89 +/- ##
=======================================
Coverage 72.19% 72.19%
=======================================
Files 13 13
Lines 838 838
Branches 116 116
=======================================
Hits 605 605
Misses 174 174
Partials 59 59 ☔ View full report in Codecov by Sentry. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR renames the exclude_interactor_variables parameter to exclude_model_variables throughout the codebase to better reflect its functionality of excluding variables from automatic formula generation (both main effects and interactions), not just interaction terms.
Changes:
- Renamed
exclude_interactor_variablestoexclude_model_variablesacross Python code, tests, and documentation - Updated help text and documentation to clarify that excluded variables can be added back as main effects using
--add_model_variables - Changed CI workflow matrix to use
macos-15instead ofmacos-13
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tfbpmodeling/tests/test_interface.py | Updated test argument name from exclude_interactor_variables to exclude_model_variables |
| tfbpmodeling/interface.py | Renamed parameter and improved help text to clarify exclusion applies to automatic formula generation |
| docs/tutorials/advanced-features.md | Updated command example to use new parameter name |
| docs/getting-started/quickstart.md | Updated example with new parameter name and added clarifying comments about usage pattern |
| docs/cli/linear-perturbation-binding-modeling.md | Updated CLI documentation with new parameter name and improved description |
| README.md | Updated main documentation with new parameter name and expanded explanations of both exclude and add model variables |
| .github/workflows/ci.yml | Updated CI matrix to use macos-15 instead of macos-13 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+101
to
104
| # drop any variables which are in args.exclude_model_variables | ||
| predictor_variables = exclude_predictor_variables( | ||
| list(predictor_variables), args.exclude_interactor_variables | ||
| list(predictor_variables), args.exclude_model_variables | ||
| ) |
There was a problem hiding this comment.
The new flag name and help text describe exclusion from 'automatic formula generation (main effects and interactions)', but here the exclusion is applied to predictor_variables immediately before interactor-term creation. If main effects are generated from a different list later, this help text becomes inaccurate. Either (a) update the help/docs to clearly state this flag affects variables used for interaction-term generation, or (b) ensure the same exclusion is applied consistently to whichever variable list drives main-effect generation too.
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.
This primarily focuses on improving the name of the exclude model variables and add model variables cmd line options