added irrchars and classes methods for tables
#40
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #40 +/- ##
=========================================
+ Coverage 7.03% 7.07% +0.04%
=========================================
Files 60 60
Lines 8870 8874 +4
=========================================
+ Hits 624 628 +4
Misses 8246 8246 ☔ View full report in Codecov by Sentry. |
|
The names are not ideal, see #41. But I don't know better names right now. |
| ``` | ||
| """ | ||
| function irrchars(t::Table) | ||
| return sum(nrchars.(Ref(t), 1:irrchartypes(t))) |
There was a problem hiding this comment.
TIL one can use Ref like that in a broadcast, nice :-)
src/Show.jl
Outdated
| @@ -49,6 +49,26 @@ function irrchartypes(t::Table) | |||
| return t.table.original_m | |||
| end | |||
|
|
|||
| """ | |||
| irrchars(t::Table) | |||
There was a problem hiding this comment.
I would just call this nrchars as well.
And irrchartypes should be nirrchartypes(or rather in OSCAR style:n_irrchartypes-- or evennumber_of_irreducible_character_types` or whatever... )
src/Show.jl
Outdated
| @@ -67,6 +87,24 @@ function classtypes(t::Table) | |||
| size(t.table, 2) | |||
| end | |||
|
|
|||
| """ | |||
| classes(t::Table) | |||
There was a problem hiding this comment.
As above I'd just call this nrclasses as well
And eventually rename both to n_classes, and classtypes to n_classtypes -- but please not in this PR, this needs to be carefully coordinated.
fingolfin
force-pushed
the
count_chars_classes
branch
from
be9ae4d
to
c850841
Compare
|
I've resolved the conflicts and updated this to look like I meant. But I am now not so sure this is a good idea. Isn't it true that some of the character/class types may not be disjoint? At least that's how I read the CHEVIE paper from 1996. ... Anyway I don't need this right now, I can try to talk with e.g. Gunter Malle about this (I want to do that anyway for a bunch of things in this code) |
I don't know, I thought that this is not the case if we consider the parameter exceptions which is done here. |
fingolfin
force-pushed
the
count_chars_classes
branch
from
c850841
to
fd1bcf4
Compare
|
Should we wait with the merge until it is clear if we are counting some characters twice? |
|
I've talked to Gunter Malle today and he thinks this should be fine, so now characters/classes are be counted multiple times. |
@fingolfin Is this what you meant in #16?