Skip to content

docs: document Cargo.toml features - #387

Merged
Mec-iS merged 2 commits into
smartcorelib:developmentfrom
teddytennant:docs/cargo-features-68
Aug 7, 2026
Merged

docs: document Cargo.toml features#387
Mec-iS merged 2 commits into
smartcorelib:developmentfrom
teddytennant:docs/cargo-features-68

Conversation

@teddytennant

Copy link
Copy Markdown
Contributor

Problem

Optional features in Cargo.toml had little or no description, so users had to dig through source or README to learn what each flag enables. Issue #68 asked for serde-style feature documentation, including the missing serde feature note.

Change

Document every feature in Cargo.toml with short comments in the style of serde's Cargo.toml:

  • default - empty (WASM-friendly defaults)
  • serde - Serialize/Deserialize for models; enables typetag on non-wasm
  • ndarray-bindings - optional ndarray DenseMatrix bindings
  • datasets - sample datasets and generators (pulls in std_rand + serde)
  • std_rand - StdRng / thread_rng instead of SmallRng
  • js - getrandom wasm_js backend for browser builds

Feature names were checked against cfg(feature = ...) usage in the crate (serde, datasets, ndarray-bindings, std_rand, js).

Test plan

  • cargo metadata --no-deps parses features successfully
  • Confirmed feature names match cfg gates in source
  • Docs-only change; no code behavior change

Fixes #68

@teddytennant
teddytennant requested a review from Mec-iS as a code owner August 7, 2026 16:06
Add serde-style comments describing each optional feature so users can
see what serde, ndarray-bindings, datasets, std_rand, and js enable
without reading the source.
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.07%. Comparing base (70d8a0f) to head (092b0b8).
⚠️ Report is 109 commits behind head on development.

Additional details and impacted files
@@               Coverage Diff               @@
##           development     #387      +/-   ##
===============================================
- Coverage        45.59%   44.07%   -1.53%     
===============================================
  Files               93       95       +2     
  Lines             8034     8055      +21     
===============================================
- Hits              3663     3550     -113     
- Misses            4371     4505     +134     

☔ 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 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Overall Assessment

The PR is well-scoped and correct — it directly addresses issue #68 with no behavioral changes. The diff is minimal and the added comments are accurate and useful. It's ready to merge with one minor suggestion.

What's Good

  • Accurate descriptions: Each comment correctly reflects what the feature does. For example, noting that datasets pulls in std_rand and serde because datasets ship serialized bundles is genuinely useful context that wasn't there before .
  • Consistent style: The serde-style inline comments above each feature (# Enable ...) are idiomatic for Rust crates and match the community convention .
  • The ### FEATURES ### banner adds visual scannability in the raw Cargo.toml file, which is a nice touch for maintainers.
  • The js feature comment is an improvement over the old one-liner — it now clarifies which wasm target and why (in-browser getrandom backend) .

Minor Suggestions

A few small improvements worth considering before merging:

  1. default = [] lacks a comment. Since the PR documents all other features, it would be consistent to add a comment like # No features enabled by default (keeps the crate WASM-friendly and dependency-light). The PR description mentions this intent but it's not reflected in the actual file .

  2. ndarray-bindings description could be slightly more specific. "DenseMatrix <-> ndarray" is good, but noting the directionality or that it's an optional interop layer (i.e., needed only when mixing ndarray types with smartcore APIs) would help users decide whether they need it.

  3. Coverage drop is unrelated but worth noting: Codecov reports a drop from 45.59% → 44.00% coverage . This is a docs-only PR adding 2 files and 21 lines — the drop is almost certainly from the base development branch being 109 commits ahead, not from this change. Worth flagging to maintainers so they're not confused.

Suggested Review Comment

You could add an inline suggestion on the default = [] line:

# No features enabled by default; keeps the build WASM-compatible
# and avoids pulling in serde/rand unless explicitly requested.
default = []

The PR is otherwise approved — it's a low-risk, high-value documentation improvement that will directly benefit new contributors trying to understand feature flags without reading source code.

Mec-iS
Mec-iS previously approved these changes Aug 7, 2026

@Mec-iS Mec-iS left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@Mec-iS
Mec-iS self-requested a review August 7, 2026 16:46
@Mec-iS
Mec-iS merged commit ecc3720 into smartcorelib:development Aug 7, 2026
13 of 14 checks passed
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.

Document features in Cargo.toml

2 participants