Skip to content

Conversation

@jorenham
Copy link
Owner

@jorenham jorenham commented Sep 5, 2025

No description provided.

Copilot AI review requested due to automatic review settings September 5, 2025 11:55
@jorenham jorenham added this to the 0.1.0 milestone Sep 5, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for the ellipj function (Jacobian elliptic functions) to the xsf library. The function takes two f64 parameters and returns four f64 values representing the sine amplitude, cosine amplitude, delta amplitude, and phase.

  • Implements the ellipj function with proper documentation and parameter handling
  • Adds test coverage for the new function using the existing xsref testing framework
  • Updates the build configuration to include the function signature

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/ellip.rs Implements the ellipj function with documentation and unsafe bindings call
src/lib.rs Exports the new ellipj function in the public API
tests/test_functions.rs Adds test macro for ellipj and creates test pattern for "dd->dddd" signature
build.rs Adds the function signature to the build configuration
Comments suppressed due to low confidence (1)

tests/test_functions.rs:1

  • This macro pattern appears to be duplicated with the existing 'dd->dddd' pattern at lines 618-628. The 'd->dddd' pattern (single input, four outputs) and 'dd->dddd' pattern (two inputs, four outputs) have identical return type tuples but different input handling. Consider whether both patterns are actually needed or if this is unintentional duplication.
//! Automatic testing of the `scipy/xsf` rust bindings using the `scipy/xsref` tables.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@jorenham jorenham enabled auto-merge September 5, 2025 11:57
@jorenham jorenham merged commit ee169d1 into master Sep 5, 2025
4 checks passed
@jorenham jorenham deleted the ellipj branch September 5, 2025 11:59
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.

1 participant