Skip to content

Conversation

@devansh2605
Copy link

@devansh2605 devansh2605 commented Oct 29, 2025

Resolves issue #580.

Edited oper.gi file to allow D ^ p (digraph and permutation) and D ^ t (digraph and transformation) to call OnDigraphs(D, p) and OnDigraphs(D, t), respectively.

@mtorpey
Copy link
Collaborator

mtorpey commented Oct 29, 2025

Thanks for the PR!

Some tests are failing here, and I think the key is that you've installed a method without first declaring the operation.

The trick to fixing this is to go into oper.gd and add a DeclareOperation statement for these two filters.

@devansh2605
Copy link
Author

devansh2605 commented Oct 30, 2025

@james-d-mitchell @mtorpey I’ve added tests for this issue. I had to adjust the formatting a few times since the GitHub checks were failing initially. The tests run correctly when I execute them manually, but I’m still getting a Syntax warning: Unbound global variable in stream:1. I’m not entirely sure what the best way to address this is (as you can see I’ve tried a few approaches).

Just to confirm for the documentation, should I add it in pkg/Digraphs/doc/oper.xml following the same structure used for the other methods in that file?

@james-d-mitchell
Copy link
Member

@james-d-mitchell @mtorpey I’ve added tests for this issue. I had to adjust the formatting a few times since the GitHub checks were failing initially. The tests run correctly when I execute them manually, but I’m still getting a Syntax warning: Unbound global variable in stream:1. I’m not entirely sure what the best way to address this is (as you can see I’ve tried a few approaches).

I've commented directly on the line that's wrong, you're reporting the exact error you'd get if you typed local some_variables; at the command line in GAP.

Just to confirm for the documentation, should I add it in pkg/Digraphs/doc/oper.xml following the same structure used for the other methods in that file?

Yes @devansh2605 that's correct.

mtorpey
mtorpey previously approved these changes Nov 10, 2025
@mtorpey
Copy link
Collaborator

mtorpey commented Nov 10, 2025

Happy with this, while noting that this currently isn't documented. I'd suggest adding a paragraph in the OnDigraphs documentation noting the new alternative syntax, since I'm not sure there's actually a man entry for a symbol like ^.

@wilfwilson wilfwilson added enhancement A label for PRs that provide enhancements. new-feature A label for new features. VIP Pull requests done by participants of the VIP programme. labels Nov 12, 2025
Copy link
Collaborator

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

I still think it would be a nice idea to, in addition, as @mtorpey suggested, add a sentence to the documentation of OnDigraphs for a digraph and a perm, and for OnDigraphs for a digraph and a transformation, to say that OnDigraphs can be used via the \^ operator.

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

Labels

enhancement A label for PRs that provide enhancements. new-feature A label for new features. VIP Pull requests done by participants of the VIP programme.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants