Bug fix: IsRightAlgebraModule is a right module, not left (DO NOT MERGE)

[fingolfin]

No description provided.

[fingolfin]

Perhaps @willemdegraaf (who wrote this code) has some insights on this? Or perhaps @hulpke or @ThomasBreuer who simply have been around for a long time and know a lot about a lot of GAP code in general?

Naively, this fix is "obviously right", but I am slightly worried that I might be missing something subtle here?

Also, I guess a test case which checks the change would be a useful addition for this PR...

[willemdegraaf]

Dear Max, Thank you for looking into this. I don't know to what problem you are referring(I am not so good at github), but I recall that at the time when I wrote the code, there were people working with bimodules, and they would be left and right modules. All the best, Willem

[fingolfin]

@willemdegraaf thanks for the quick reply!

OK, I see now: IsAlgebraModule is defined as a property of objects in the filter IsLeftModule., and then both IsLeftAlgebraModule and also, confusingly, IsRightAlgebraModule are properties for objects in the filter IsLeftModule.

Perhaps IsLeftAlgebraModule really should be a synonym for IsAlgebraModule and IsLeftModule? But as I wrote, IsAlgebraModule is right now only defined for IsLeftModule anyway...

Then again, to the best of my knowledge, neither IsLeftAlgebraModule nor IsRightAlgebraModule are ever used: Other than in the definition, the two don't occur elsewhere, neither in the GAP library nor in any deposited package.

So perhaps we should just remove them: they are unusable anyway, as no methods or implications are installed for them

[fingolfin]

Note that e.g. IsRightAlgebraModuleElement, as well as the constructors RightAlgebraModuleByGenerators and RightAlgebraModule, are used.

So perhaps that they are not set is just an oversight?

Note that the property IsAlgebraModule also has no method or implications installed, so the only way it is set on something are explicit SetIsAlgebraModule calls, which is what the code does.

But then, it seems to me that IsAlgebraModule, IsRightAlgebraModule and IsLeftAlgebraModule then perhaps should not be properties, but rather categories -- you can't sensibly take some left module, and "test" if it is also a right module, can you? (I mean, sure, if e.g. the acting domain is commutative, then one could rightfully treat this as a right module, even a bi-module -- but that's not what I think IsRightAlgebraModule, IsRightAlgebraModuleElement etc. are meant to indicate -- rather, they are technical flags which indicate that "multiplication from the right" is supported, which a pure left-module element might not, even if the acting domain is abelian.

Hum.

[willemdegraaf]

Dear Max, I do not have the possibility to dive into it now, but I would think that you are right in saying that these things should be categories. All the best, Willem

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@144b52a). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #2221   +/-   ##
=========================================
  Coverage          ?   93.65%           
=========================================
  Files             ?      708           
  Lines             ?   803466           
  Branches          ?        0           
=========================================
  Hits              ?   752495           
  Misses            ?    50971           
  Partials          ?        0           

Impacted Files	Coverage Δ
lib/algrep.gd	100.00% <ø> (ø)