Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify the interface to linking/core/threshold.py #174

Open
riley-harper opened this issue Dec 5, 2024 · 0 comments
Open

Simplify the interface to linking/core/threshold.py #174

riley-harper opened this issue Dec 5, 2024 · 0 comments
Labels
component: core
Milestone
v4.0.0

Comments

@riley-harper
Copy link
Contributor

Instead of accepting the entire training configuration in predict_using_thresholds, we would be better off just accepting a single decision argument, since that's the only thing that the function extracts from the configuration. This module would also be a great place to add type hints and improve comments and documentation.

This is a breaking change since it changes one of the arguments of a function.
@riley-harper riley-harper added this to the v4.0.0 milestone Dec 5, 2024
riley-harper added a commit that referenced this issue Dec 5, 2024
@riley-harper
[#174] Add type hints to linking.core.threshold
49bda13
riley-harper added a commit that referenced this issue Dec 5, 2024
@riley-harper
[#174] Add a couple of unit tests for linking.core.threshold
28bcd03
riley-harper added a commit that referenced this issue Dec 5, 2024
@riley-harper
[#174] Pass just decision into predict_with_thresholds() instead of t…
ad6ce10 
…he whole training config

This makes it clear which part of the config predict_with_thresholds() is using
and makes it easier to call. It also means that predict_with_thresholds() does
not need to know about the structure of the config.
riley-harper added a commit that referenced this issue Dec 5, 2024
@riley-harper
[#174] Do some minor refactoring and cleanup of linking.core.threshold
5424513
riley-harper added a commit that referenced this issue Dec 5, 2024
@riley-harper
[#174] Replace a SQL query with the equivalent spark expression
dd16360 
This prevents a possible SQL injection error by setting alpha_threshold to
something weird.  It's also a bit easier to read and work with in my
experience. It's more composable since you can build up the expression instead
of having to write all of the SQL at once.
riley-harper added a commit that referenced this issue Dec 5, 2024
@riley-harper
[#174] Rewrite some thresholding code to use PySpark exprs instead of…
647a751 
… SQL
riley-harper added a commit that referenced this issue Dec 6, 2024
@riley-harper
[#174] Use withColumn() instead of select("*", ...)
b5c8ae9 
This is just a bit cleaner to read, and makes clear the names of the columns
that we're adding. We can't select ratio and prediction at once because
prediction depends on ratio.
riley-harper added a commit that referenced this issue Dec 6, 2024
@riley-harper
[#174] Improve the error message when there's no probability column
1ffb6d1
riley-harper added a commit that referenced this issue Dec 6, 2024
@riley-harper
[#174] Update documentation and add a few logging debug statements
d32c2bf
@riley-harper riley-harper mentioned this issue Dec 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: core
Projects
None yet
Milestone
v4.0.0
Development

No branches or pull requests
1 participant
@riley-harper