Repository navigation
Every verdict is spelled passes, refuses to stand in for a bool, and comes from a function whose verb says what it judges - #977
Conversation
|
Sorry @jmrplens, your pull request is larger than the review limit of 150,000 diff characters |
|
Important Review skippedToo many files! This PR contains 227 files, which is 127 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. ⚙️ Run configuration
⛔ Files ignored due to path filters: 4
📒 Files selected for processing: 227
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
passes, refuses to stand in for a bool, and comes from a function whose verb says what it judges
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## fix/one-vocabulary-per-choice #977 +/- ##
=================================================================
+ Coverage 96.88% 96.90% +0.01%
=================================================================
Files 445 445
Lines 89986 90395 +409
=================================================================
+ Hits 87185 87594 +409
Misses 2801 2801 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Numerical conformance1893/1893 checks pass across 111 domains and 523 standards (227 normative designations, 125 further published sources). Used in the tables below is how much of that clause's published tolerance the deviation consumes: 100 % sits exactly on the limit, 5 % uses a twentieth of the allowance, and a dash means the clause states no two-sided tolerance for the quantity, so there is no budget to spend. It is reported and never used to decide a verdict, which is settled at full precision before any rounding. Renamed (4)
Closest to their published limit (top 5) The rows with the least room left, so the ones a change is most likely to push over.
|
…d comes from a function whose verb says what it judges A pass or a fail is spelled `passes` on every verdict of the library. `DBHRCheck`, `DBHRAssessment`, `ActivityAssessment`, `PeriodAssessment`, `PeopleAssessment` and `RailwayChange` said `complies`, `MountingCheck` said `acceptable`, `CeilingSpecimenCheck`, `BackgroundMarginCheck`, `SourcePositionCheck` and the prediction detail check said `satisfied`, `BandFlatnessCheck` said `all_satisfied`, and the enclosure test environment said `applicable`; all of them now say `passes`. The per-octave mask of `BandFlatnessCheck` is `within_limit`, named like the per-band `within_tolerance` of the instrument verifications. A flag that is not a pass or a fail keeps its own name with its reason: `within_guideline`, because DIN 4150-3 5.1 does not call exceeding a guideline value a failure, `significant`, a reason to investigate, and `qualified`.
d394c8b to
b2cc946
Compare
[Medium impact] Renames verdict attributes and functions across the codebase. The PR appears safe to merge, with a non-blocking fix needed for custom drawing order in the tonal survey plot. Findings
Reviews (1) · Last reviewed commit: "Every verdict is spelled `passes`, refus..." · Reviewed by Greptile |
| style_default(kwargs, "color", _C_PRIMARY) | ||
| kwargs.setdefault("width", 0.66) | ||
| kwargs.setdefault("label", _t(_BAND_LEVEL_LABEL, language)) | ||
| ax.bar(positions, levels, zorder=3, **kwargs) |
There was a problem hiding this comment.
Custom drawing order breaks plotting
TonalSurveyAssessment.plot(zorder=2) raises TypeError instead of drawing the spectrum. The public method forwards drawing options through kwargs, but this call also supplies zorder=3 explicitly. Set the default with kwargs.setdefault("zorder", 3) and pass it only once.
| ax.bar(positions, levels, zorder=3, **kwargs) | |
| kwargs.setdefault("zorder", 3) | |
| ax.bar(positions, levels, **kwargs) |
There was a problem hiding this comment.
Fixed in #992, commit a7a1d07 (with a refactor of the gate in c801d57). The same shape was in 71 calls across 24 modules; each now merges its default through styled(kwargs, ...), and scripts/check_plot_style_defaults.py refuses a keyword written beside the function's own **kwargs. A test calls TonalSurveyAssessment.plot(zorder=2) and checks the bars; the figures are unchanged.
|



What changes for you
A pass or a fail is spelled
passeson every verdict of the library.DBHRCheck,DBHRAssessment,ActivityAssessment,PeriodAssessment,PeopleAssessmentandRailwayChangesaidcomplies,MountingChecksaidacceptable,CeilingSpecimenCheck,BackgroundMarginCheck,SourcePositionCheckand the prediction detail check saidsatisfied,BandFlatnessChecksaidall_satisfied, and the enclosure test environment saidapplicable; all of them now saypasses. The per-octave mask ofBandFlatnessCheckiswithin_limit, named like the per-bandwithin_toleranceof the instrument verifications. A flag that is not a pass or a fail keeps its own name with its reason:within_guideline, because DIN 4150-3 5.1 does not call exceeding a guideline value a failure,significant, a reason to investigate, andqualified.A verdict no longer stands in for a bool. A record a
verify_,assess_orcheck_function returns, and every record with apasses, raisesTypeErroronif verdict:and the message names the attribute to read. Before, a dataclass was always true, soif result:passed every failure:QuasiPeakDynamicsResult,AircraftSystemComplianceResult,WeightingVerification,PhaseVerification,SignalBurstVerificationandVibrationMeterVerification(read.passes),FilterComplianceResult,WeightingComplianceResultandIntensityInstrumentComplianceResult(read.overall_class),DamageAssessmentandAssessmentVelocity(read.within_guideline),PrecisionCriteria(read.qualified),TonalAssessmentResult(read.adjustment),ToneAudibilityResult(read.decisive_audibility) andBackgroundNoiseAssessment(read.category).A judgement carries its verb and returns a record that keeps its reasons.
vibration.assess_operational_changeholds the change and the zone B limit and reads the threshold andsignificantfrom them.materials.check_limp_framereturns aLimpFrameCheckwith the ratio, the threshold andpasses.room.assess_perceptible_differencereturns aPerceptibleDifferencewith the just-noticeable difference anddifferent.noise_control.check_enclosure_test_environmentreturns anEnclosureTestEnvironmentCheck, so no public name starts withtestand pytest no longer collects one from a user's test module; a room ISO 11546-2 sets no K2 limit for still passes withrequired_area_ratioatNone.environment.check_short_distancereturns aShortDistanceCheckthat holds the five lengths and reads the three ratios of ISO 10847 6.3.1 and thebeforeandafteranswers from them.room.check_prediction_detailreturns aPredictionDetailCheck.building.assess_requirement_conformityreturns aRequirementConformitywith the settledmargin_dband a strictpasses, as ISO 12999-1 Formulae (4) and (5) print it.building.check_strong_couplingreturns aStrongCouplingCheckwithholdsper band, thethreshold_dbandpassesover every band, and its documentation now asks for the one-wayDv,ijthe formula compares, not the direction average: the threshold changes when the two elements swap.environment.assess_tonal_surveyreturns aTonalSurveyAssessmentwith the prominent bands, the level differences they are read from and a.plot().vibration.verify_rigid_mass_calibrationjudges a measuring chain, so its verb isverify_.hearing.FixtureIsolationVerificationis the recordverify_fixture_isolationreturns.FieldIndicators.check_field_stationarity()returns aFieldStationarityCheckwithstationaryband by band andpassesover them, read against the 0.6 of ISO 9614-1 Table B.3.materials.check_base_plate_scatteringreturns aBasePlateScatteringCheckinstead of a tuple of the bands over the limit. It holds the eighteen bands of ISO 17497-1 Table 1 and the measured coefficients, read-only, and reads the limits, the settled margins, the strictexceedsper band, theexceeded_frequenciesandpassesfrom them. Its.plot()draws the coefficients against the stepped limit, on an axis labelled with the full-scalef/Nthe table is keyed to, with the bands over the limit ringed, and the diffuser guide shows it. The guide's citation of Clause 6.4 for the base plate limits is Clause 6.2.The three background corrections that returned a value and a flag return a small record each:
building.background_corrected_levelaBackgroundCorrectedLevel(corrected_levels,limitedandmargins_db, also for a positions by bands array),emission.background_noise_correction_at_workstationaWorkstationBackgroundCorrection(correctionandupper_bound, read from the margin and the grade) andnoise_control.extraneous_corrected_mean_levelanExtraneousCorrectionResult(the corrected level, the margin of the two energy means andcapped). Each reads its limits from its own module and warns as before.What breaks
Every rename is a hard break with no alias; the migration guide (
docs/start/upgrading.mdand both site copies) has a new section, "A verdict sayspasses, refuses a truth value, and comes from a verb", with a row for each name that shipped in 3.3.0 or 4.0.0rc1, and the CHANGELOG lists the rest..compliesonDbHrCheck,DbHrAssessment(nowDBHRCheck,DBHRAssessment),ActivityAssessmentandPeriodAssessment: read.passes.if result:on any verdict raisesTypeError: read the attribute the message names.vibration.is_significant_change(change, zone_b_upper)isvibration.assess_operational_change(change, zone_b_upper).significant.materials.limp_frame_applicable(k_c, criterion=...)ismaterials.check_limp_frame(k_c, criterion=...).passes. A modulus ratio exactly on the threshold no longer passes: Allard & Atalla print both criteria as strict inequalities (|Kc/Kf| < 0.05and< 0.2).fi.field_is_stationary(limit=...)isfi.check_field_stationarity().passes, and band by band its.stationary; the limit is the 0.6 of Table B.3, read as.limit, and no longer an argument.room.perceptibly_different(symbol, first, second)isroom.assess_perceptible_difference(symbol, first, second).different, and two values of different shapes raiseValueErrorinstead of broadcasting.materials.check_base_plate_scattering(s) == ()ismaterials.check_base_plate_scattering(s).passes, with the bands over the limit in.exceeded_frequencies. A coefficient that is not finite raisesValueError, where a NaN used to pass.corrected, limited = building.background_corrected_level(lp, lb)isresult.corrected_levelsandresult.limited;k1, held = emission.background_noise_correction_at_workstation(lp, lb)isresult.correctionandresult.upper_bound.rigid_mass_calibration_check,satisfies_lower_requirement,satisfies_upper_requirement,strong_coupling_satisfiedandtonal_seeking_surveywere exported by the 3.3.0 top level and are gone:vibration.verify_rigid_mass_calibration,building.assess_requirement_conformity(..., bound="minimum"|"maximum").passes,building.check_strong_coupling(...).holdsandenvironment.assess_tonal_survey(...).prominent. The counts at the top of the migration guide are re-measured: seventy-four of the 885 names the 3.3.0 top level exported are gone.noise_control.test_environment_applicability,room.detail_is_sufficient,environment.is_short_distance,hearing.FixtureIsolationCheckand the flagssatisfied,all_satisfied,acceptableandapplicable) are recorded in the CHANGELOG.How it was verified
Every threshold a new record reads was checked on the printed page, not from memory: ISO 12999-1:2020 Formulae (4) and (5) (PDF page 17, folio 11), both strict; ISO 10847:1997 6.3.1 (PDF page 10, folio 6), strict at 0,1, and 8.1.4 (PDF page 14, folio 10), where 250 m is only the distance past which more than three repetitions may be needed, which the API table now says; ISO 17497-1:2004+A1:2014 6.2 and Table 1 (PDF page 13, folio 5), keyed by
f/N; ISO 10848-1:2017 Formula (4) (PDF page 15, folio 9) and BS EN ISO 10848-1:2006 Formula (15) (PDF page 15, folio 7), which print the one-wayDv,ijwhile Formula (14) on the same page prints the direction average; Allard & Atalla, Propagation of Sound in Porous Media 2e, PDF pages 259 and 260 (printed 253 and 254), which print|Kc/Kf| < 0.05,< 0.2and "lower than 20 kPa"; and BS EN ISO 11820:1997 Equations (17) and (18) (PDF pages 18 and 19, folios 10 and 11), whose corrected meanL_p1/L_pIthe extraneous correction now names apart from the measured mean of Equation (2) its margin is read from.New tests pin each record at its edges: the strong-coupling threshold for unlike elements (a ratio of 2 gives 3 - 10 lg 2 dB, one band either side of it fails the verdict, and swapping the elements gives 3 + 10 lg 2 dB); a geometry that is short both before and after the barrier (ratios 0,2 and 0,5) and one with unequal sides; the workstation background at exactly the 6 dB and 3 dB floor of each grade, on decimal pairs that land either side in binary, which hold nothing and do not warn; the limp-frame ratio exactly on each printed threshold, which does not pass, and a millionth inside it, which does; every refusal of
BasePlateScatteringCheck(rank, length, a band set other than Table 1's, NaN and infinities) and ofRequirementConformity,ExtraneousCorrectionResult,PerceptibleDifferenceandFieldStationarityCheck; and the derived values of the records (OperationalChange.thresholdof 1,125 mm/s for a 4,5 mm/s zone B, the 10 dB extraneous margin, the per-positionmargins_db, the limp-frame ratio and threshold). The base plate figure was drawn again in both languages and both themes and passes the figure checks. The conformance report keeps its numbers (1893 of 1893 checks), with the ISO 10847 row now also asking for a geometry that is short.Gates on this branch:
ruff check .andruff format --check .clean,mypy src scriptswith no issues in 700 files,bandit -q -r srcclean, everyscripts/check_*.pythat reads the tree green (the clipanim_epnl_flyoveris stale for a reason outside this change), the API reference covering all 3073 public names, and the full suite green (28880 passed, 69 skipped). The documentation snippets run clean (7545 blocks over 731 pages), and the site installs from its lockfile, type-checks, builds and passes html-validate.The guard
tests/test_verdict_shape.pyreads the whole public API, the public methods of every published class included, and holds four rules: no public name reads as a test; a function or a method that answers with a bool, a tuple of them, a boolean array or a verdict carriesverify_,assess_orcheck_, and each of those returns a record; every verdict a judging verb returns, and every record with a scalarpasses, has a__bool__that raisesTypeError, checked in the source and then on real instances; and no verdict spells its overall flagcomplies,satisfied,all_satisfied,acceptableorapplicable. The two regime predicates (low_frequency_procedure_applies,resolve_tones_separately) and the methods that test membership or provenance, look up one requirement of a verdict by its clause, or say how a value is reported are keyed by location with their reason. CONTRIBUTING states the rule.Red on the base of this branch (65 failed, 70 passed), excerpts:
plus 24 failures of
bool()on a bare instance of each verdict class and 33 on real verdicts built through the public calls, each of which either did not raise (Failed: DID NOT RAISE TypeError) or had no judging verb to be built with yet. Green on this branch: 147 passed intests/test_verdict_shape.py.