Skip to content

Conversation

@rgabhi2526
Copy link

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

Closes #40806

Description

This PR addresses Issue #40806 by adding support for the implementation keyword argument to the FiniteField (or GF) constructor, treating it as an alias for the existing impl keyword.

The change is implemented in src/sage/rings/finite_rings/finite_field_constructor.py within the FiniteFieldFactory.create_key_and_extra_args method.

Changes Made:

  1. Added implementation=None to the function signature.
  2. Added logic to check for both impl and implementation.
  3. Raises a ValueError if both keywords are used simultaneously.
  4. If only implementation is used, its value is assigned to impl to ensure consistency with the existing factory key and implementation logic.
  5. A doctest was added to verify both the new functionality and the conflict check.

Reviewer Checklist:

  • Verify that GF(q, impl='...', implementation='...') raises a ValueError.
  • Review the change in create_key_and_extra_args to ensure backward compatibility is maintained (i.e., existing uses of impl still work).

@vincentmacri
Copy link
Member

Thank you for your contribution, but someone else has already submitted #41189 to fix this. #41189 is a bit further along already, but we can leave this PR open for now just in case progress on #41189 stalls for some reason.

@r-mb
Copy link
Contributor

r-mb commented Nov 19, 2025

See also #35609.

@vincentmacri
Copy link
Member

See also #35609.

Good catch. I'll need to compare them.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support implementation=* in addition to impl=* in FiniteField

3 participants