Skip to content

refactor(naive_bayes): remove duplicated RealNumber trait bound - #423

Merged
Mec-iS merged 1 commit into
smartcorelib:developmentfrom
seroze:fix/duplicate-realnumber-bound
Aug 10, 2026
Merged

refactor(naive_bayes): remove duplicated RealNumber trait bound#423
Mec-iS merged 1 commit into
smartcorelib:developmentfrom
seroze:fix/duplicate-realnumber-bound

Conversation

@seroze

@seroze seroze commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #424

Checklist

  • My branch is up-to-date with development branch.
  • Everything works and tested on latest stable Rust. (rustc 1.97.1)
  • Coverage and Linting have been applied

Current behaviour

GaussianNB asks for TX: Number + RealNumber + RealNumber. RealNumber is listed twice, and it happens in four places: the struct itself, plus its Display, SupervisedEstimator and Predictor impls.

Saying it twice doesn't ask anything extra of TX, so nothing is broken here. rustc compiles it without a word, and clippy has nothing to say about it either, which is presumably how it survived this long.

It looks like it crept in by accident. Three of the four came with the v0.4 generics rewrite (52eb6ce), where the old single T: RealNumber was split into TX/TY. The fourth was copy-pasted a few days later along with the new Display impl (ba70bb9).

New expected behaviour

Just TX: Number + RealNumber. Same requirements on TX, same generated code, one less thing to read.

Testing

No new tests, since there's no behaviour change to assert and no new code paths for coverage to reach. The existing naive_bayes tests still pass:

  • cargo build — clean
  • cargo clippy --all-features -- -D warnings — clean
  • cargo test naive_bayes — 23 passed, 0 failed

Change logs

Changed

  • Removed the repeated RealNumber bound from GaussianNB and its Display, SupervisedEstimator and Predictor impls.

`GaussianNB` declared `TX: Number + RealNumber + RealNumber` in four
places (the struct and its Display, SupervisedEstimator and Predictor
impls). The repeated bound is a no-op, so this is purely cosmetic.

rustfmt collapses the now-shorter generic lists onto one line.
@seroze
seroze requested a review from Mec-iS as a code owner August 10, 2026 00:26
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.64%. Comparing base (70d8a0f) to head (15cd811).
⚠️ Report is 123 commits behind head on development.

Additional details and impacted files
@@               Coverage Diff                @@
##           development     #423       +/-   ##
================================================
+ Coverage        45.59%   63.64%   +18.05%     
================================================
  Files               93       95        +2     
  Lines             8034     8142      +108     
================================================
+ Hits              3663     5182     +1519     
+ Misses            4371     2960     -1411     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Mec-iS
Mec-iS merged commit 6bd1afe into smartcorelib:development Aug 10, 2026
15 checks passed
@Mec-iS

Mec-iS commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

thanks 👍

@Mec-iS

Mec-iS commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

@seroze if you are interested in mechanistic interpretability, check out my post https://www.tuned.org.uk/posts/019_arrowspace_local_minima_walkthrough/ and arrowspace

@seroze

seroze commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@seroze if you are interested in mechanistic interpretability, check out my post https://www.tuned.org.uk/posts/019_arrowspace_local_minima_walkthrough/ and arrowspace

Thanks man, will check this. I'm also planning to apply for https://sparai.org/projects/f26/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicated RealNumber trait bound in GaussianNB

2 participants