Skip to content

Cleanup linear_algebra#1890

Open
schmidma wants to merge 16 commits intoHULKs:mainfrom
schmidma:dont-hesitate-to-maybe-overcompensate
Open

Cleanup linear_algebra#1890
schmidma wants to merge 16 commits intoHULKs:mainfrom
schmidma:dont-hesitate-to-maybe-overcompensate

Conversation

@schmidma
Copy link
Copy Markdown
Member

@schmidma schmidma commented Jun 18, 2025

Why? What?

This PR prepares linear_algebra to be published, cleaning up structure and some naming, as well as providing feature flags and documentation.

fixes #1893

@github-project-automation github-project-automation Bot moved this to Request for Review in Development Jun 18, 2025
Comment thread crates/linear_algebra/src/lib.rs
Comment thread crates/linear_algebra/src/framed.rs Outdated
Comment thread crates/linear_algebra/Cargo.toml
Comment thread crates/linear_algebra/Cargo.toml Outdated
@schmidma schmidma force-pushed the dont-hesitate-to-maybe-overcompensate branch from 8561922 to 1e86b6e Compare June 23, 2025 08:55
@schmidma schmidma enabled auto-merge June 23, 2025 09:05
@schmidma schmidma force-pushed the dont-hesitate-to-maybe-overcompensate branch 2 times, most recently from 0f8e9a7 to b60caf9 Compare September 18, 2025 16:45
Comment thread crates/linear_algebra/src/orientation.rs Outdated
Comment thread crates/linear_algebra/src/transform.rs Outdated
@schmidma schmidma force-pushed the dont-hesitate-to-maybe-overcompensate branch from 6e0a895 to 114d6fe Compare March 14, 2026 21:20
@schmidma
Copy link
Copy Markdown
Member Author

I gave this another bump, review, cleanup and rebase...

Copy link
Copy Markdown
Member

@rmburg rmburg left a comment

Choose a reason for hiding this comment

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

only a few nitpicks, otherwise very nice changes! Looking forward to this on crates.io :)

Comment thread crates/bevyhavior_simulator/src/bin/intercept_ball.rs Outdated
Comment thread crates/linear_algebra/src/framed.rs Outdated
@schmidma schmidma force-pushed the dont-hesitate-to-maybe-overcompensate branch from 9827e0c to 353d903 Compare March 16, 2026 13:17
Comment on lines +16 to +22
/// # Example
/// ```rust
/// use linear_algebra::{point, Point2};
///
/// struct World;
/// let p: Point2<World> = point![1.0, 2.0];
/// ```
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add another example that specifies the frame within the macro.

let p = point![<World>, 1.0, 2.0];

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The whole point (heh) of being able to specify the frame inside the macro was being able to omit more verbose type information elsewhere.

Comment on lines +15 to +21
/// # Example
/// ```rust
/// use linear_algebra::{vector, Vector2};
///
/// struct World;
/// let v: Vector2<World> = vector![1.0, 2.0];
/// ```
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add another example that specifies the frame within the macro.

let p = vector![<World>, 1.0, 2.0];

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

Comment thread crates/linear_algebra/src/isometry.rs Outdated
Comment thread crates/linear_algebra/src/isometry.rs Outdated
//! - Wraps commonly used parts of the [`nalgebra`] API to provide frame-safe abstractions.
//! - Supports 2D and 3D geometry with extensible coordinate system tagging.
//! - Provides clear and explicit geometric transformations.
//! - Re-exports [`nalgebra`] so `point!` and `vector!` work without a direct dependency.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this a "feature"?
I think its an implementation detail. It would work fine too if instead of wrapping nalgebra's macros we had copied them instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

rephrased some of the docstrings

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm still not convinced the re-exporting is a feature worthy of being listed here.

Comment thread crates/linear_algebra/src/lib.rs Outdated
Comment thread crates/linear_algebra/src/lib.rs Outdated
Comment thread crates/linear_algebra/src/lib.rs
Comment thread crates/linear_algebra/src/pose.rs Outdated
Comment thread crates/linear_algebra/src/pose.rs Outdated
Comment on lines +214 to +215
assert!((isometry.angle() - 0.5).abs() < 1.0e-6);
assert!((isometry.rotation().angle() - 0.5).abs() < 1.0e-6);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems like a job for approx

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

Labels

None yet

Projects

Status: Review

Development

Successfully merging this pull request may close these issues.

Fix Isometry API inconsistency

4 participants