metrics.py: small code quality improvements #63
Closed
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
TaXxER
requested review from
Lorenzo-Perini,
danielha1 and
flinder
as code owners
meta-cla
bot
added
the
CLA Signed
Summary: Code quality improvements for metrics.py following Google Python style guide: **Type annotations:** - Removed unused imports (`Dict`, `Tuple` from typing) - Fixed `**kwargs` type hint from `Dict[str, Any]` to `Any` (the correct type for kwargs values) - Updated `Tuple` to lowercase `tuple` for Python 3.9+ style **Replaced assert statements with proper exceptions:** - Production code should not use `assert` for validation since asserts can be disabled with `-O` flag - Converted 5 `assert` statements to `raise ValueError()` or `raise AssertionError()` in: - `multi_cg_score`: metric validation - `kuiper_distribution`: input validation for positive x - `rank_multicalibration_error`: segment weight validation - `calibration_free_normalized_entropy`: labels shape validation - `wrap_multicalibration_error_metric`: segment columns and metric version validation **Fixed variable shadowing:** - `calibration_ratio` variable shadowed the function → renamed to `ratio` - `fpr` variables in `fpr_with_mask` and `fpr_at_precision` shadowed the function → renamed to `false_positive_rate` - `normalized_entropy` variable shadowed the function → renamed to `ne` **Extracted magic numbers to module-level constants:** - Added `KUIPER_STATISTIC_MAX` and `KUIPER_STATISTIC_MIN` as documented constants - Removed duplicated local definitions inside `kuiper_test()` **Test cleanup:** - Removed debug `print()` statement from test Reviewed By: Lorenzo-Perini Differential Revision: D89555477
facebook-github-bot
force-pushed
the
export-D89555477
branch
from
13ff434 to
609b487
Compare
|
This pull request has been merged in 516d884. |
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.
Summary:
Code quality improvements for metrics.py following Google Python style guide:
Type annotations:
Dict,Tuplefrom typing)**kwargstype hint fromDict[str, Any]toAny(the correct type for kwargs values)Tupleto lowercasetuplefor Python 3.9+ styleReplaced assert statements with proper exceptions:
assertfor validation since asserts can be disabled with-Oflagassertstatements toraise ValueError()orraise AssertionError()in:multi_cg_score: metric validationkuiper_distribution: input validation for positive xrank_multicalibration_error: segment weight validationcalibration_free_normalized_entropy: labels shape validationwrap_multicalibration_error_metric: segment columns and metric version validationFixed variable shadowing:
calibration_ratiovariable shadowed the function → renamed toratiofprvariables infpr_with_maskandfpr_at_precisionshadowed the function → renamed tofalse_positive_ratenormalized_entropyvariable shadowed the function → renamed toneExtracted magic numbers to module-level constants:
KUIPER_STATISTIC_MAXandKUIPER_STATISTIC_MINas documented constantskuiper_test()Test cleanup:
print()statement from testDifferential Revision: D89555477