Skip to content

Conversation

@cxzhong
Copy link
Contributor

@cxzhong cxzhong commented Nov 18, 2025

Updated error handling for morphism composition to provide clearer messages regarding codomain and domain compatibility.
Fix #40391
I think it may be strict in mathematical definition. But it is useful to prevent order mismatch. to get some error results in this issue.
And for Words("ab"), we think b>a, for Words("ba"), we think a>b, so they are different order in sage's words system.
We prevent words order mismatch behavior to output some mathematical incorrect output

📝 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

Updated error handling for morphism composition to provide clearer messages regarding codomain and domain compatibility.
@cxzhong cxzhong requested a review from Copilot November 18, 2025 14:36
@cxzhong cxzhong changed the title Added domain/codomain validation in the __mul__ method Added domain/codomain validation in the __mul__ method in words Nov 18, 2025
Copilot finished reviewing on behalf of cxzhong November 18, 2025 14:41
Refactor composition validation logic in morphism.py.
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.

Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

src/sage/combinat/words/morphism.py:893

  • This method raises ValueError - should raise an ArithmeticError or return NotImplemented instead.
    This method raises ValueError - should raise an ArithmeticError or return NotImplemented instead.
    This method raises ValueError - should raise an ArithmeticError or return NotImplemented instead.
    def __mul__(self, other):

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cxzhong cxzhong closed this Nov 19, 2025
@cxzhong cxzhong reopened this Nov 19, 2025
@cxzhong cxzhong changed the title Added domain/codomain validation in the __mul__ method in words Prevent order mismatch in words function Nov 19, 2025
Updated error messages to provide clearer feedback when the codomain alphabet is not included in the domain alphabet with the correct ordering.
@cxzhong cxzhong requested review from dimpase and orlitzky November 19, 2025 07:47
@cxzhong
Copy link
Contributor Author

cxzhong commented Nov 19, 2025

We should put the _is_alphabet_included_with_order logic in this or the alphabet file?

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.

contradiction in the behavior of WordMorphism

1 participant