Skip to content

Primal trait#131

Open
FireNameFN wants to merge 6 commits into
kmolan:mainfrom
FireNameFN:primal
Open

Primal trait#131
FireNameFN wants to merge 6 commits into
kmolan:mainfrom
FireNameFN:primal

Conversation

@FireNameFN

Copy link
Copy Markdown

What & why

What:
Implemented Primal trait as requested in #123. Description copied from Plottable. Tests are questionable. Implementations of Primal trait are in primal module.

Why:
I got bored so contributed to Open Source.

Closes #123

Checklist

  • CHANGELOG.md updated under ## [Unreleased] (required for behavior-facing changes)
  • cargo test + cargo clippy --all-targets clean locally
  • New public APIs have a doc example
  • No unwrap/expect/panic on library paths (typed errors instead)

@FireNameFN FireNameFN requested a review from kmolan as a code owner July 12, 2026 03:36

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the first PR! 🎉 You'll get a first review within 48 h. CI runs the 5-target matrix — a budget-gate failure isn't your fault, see ci/README.md.

Comment thread showcase/viz/src/sink.rs
fn scalar_of(&mut self, path: &str, value: impl Plottable) -> Result<(), VizError> {
self.scalar(path, value.to_plot_f64())
/// Logs any [`Primal`] scalar without an explicit `to_f64`.
fn scalar_of(&mut self, path: &str, value: impl Primal) -> Result<(), VizError> {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Maybe

Suggested change
fn scalar_of(&mut self, path: &str, value: impl Primal) -> Result<(), VizError> {
fn scalar_of(&mut self, path: &str, value: &impl Primal) -> Result<(), VizError> {

But I don't know about compatibility.

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.

Keep impl Primal (by value). to_f64 already takes &self, so this is compatible with how callers used Plottable, no need to change it.

@kmolan

kmolan commented Jul 12, 2026

Copy link
Copy Markdown
Owner

@rtmongold could you take the first pass on reviewing?

@rtmongold

rtmongold commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Thanks @FireNameFN for the first contribution —welcome! I left a review with a few fixes that need to be applied.

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.

Add a Primal scalar-projection trait

3 participants