Oscar.jl icon indicating copy to clipboard operation
Oscar.jl copied to clipboard

Consistency: add `snake_case` aliases for all (? most?) `CamelCaseConstructors`

Open fingolfin opened this issue 2 years ago • 3 comments

For end users, it is confusing. When to write MatrixGroup, and when matrix_group? When CyclotomicField and when cyclotomic_field ? Same for ideal, matrix, and many others.

Of course "experts" know that CamelCase indicates a typename and hence the name of a constructor. But even for them it can be challenge to know when to use which, esp. in the many cases were already both names exist, with a few "shared" methods but each having separate methods as well....

I discussed this with @fieker and we think that we should shield end users from having to learn complicated rules for these things as much as possible. Of course there are things where you just have to deal with types, and that's fine. But where it is possible to avoid the ambiguity, we should do so by providing the snake_case name, too.

Moreover, this principle should be explicitly documented in the styleguide section of the Oscar manual.

Here is an incomplete list of type names where this should be addressed, to be extended

  • [ ] CycloctomicField
  • [ ] Ideal
  • [ ] MatrixGroup
  • [ ] Matrix
  • [ ] PermGroup
  • [ ] PolynomialRing

fingolfin avatar Jun 09 '22 11:06 fingolfin

I agree and just want to note that for some of these cases we can do aliases, but I would not want to do it for the types. I don't think we want matrix_group === MatrixGroup. (While we are at it, it might be a good idea to untangle the type constructors and the user facing constructors).

thofma avatar Jun 09 '22 12:06 thofma

As someone who's trying to write a package using Oscar which implements the creation of an Oscar matrix group from data of a different package (in this case Gapjm): is it my duty to implement both MatrixGroup(X::Gapjm.PRG) and matrix_group(X::Gapjm.PRG) as well? Of course it's my package so I decide what to do but I'd follow Oscar guidelines and then I feel it's annoying implementing both all the time. Also, this feels like a general Julia problem. Isn't there a guideline/solution already?

ulthiel avatar Jun 09 '22 18:06 ulthiel

The way the aliases are setup, you can do both. But I think we should suggest overloading not the alias version. I hope that we are moving to a snake_case world (except for type names), so that in the future the rule is to always use snake_case version.

P.S.: The aliases are identical as methods (===). If you try to define both, you get a Method overwritten warning (not in the REPL, but in a module).

thofma avatar Jun 09 '22 19:06 thofma

FiniteField has no snake_case equivalent like finite_field as well.

lgoettgens avatar Aug 15 '23 09:08 lgoettgens

See https://github.com/Nemocas/Nemo.jl/pull/1543 for treatment of finite_field

fingolfin avatar Sep 27 '23 08:09 fingolfin

Ideal seems to have already been changed somewhere. The only occurrences of calls in Oscar, I can see as AlgebraicSolving.Ideal and Singular.Ideal

lgoettgens avatar Sep 27 '23 08:09 lgoettgens

I believe this one is essentially resolved. If any related issues remain, they should be submitted as new issues with a detailed description of what the problem is.

fingolfin avatar Sep 29 '23 14:09 fingolfin