LieAlgebras: Streamline exports in Oscar.LieAlgebras
#4036
Conversation
lgoettgens
added
topic: LieAlgebras
experimental
lgoettgens
force-pushed
the
lg/Lie-exports
branch
from
d27e6c1
to
b626ba8
Compare
lgoettgens
force-pushed
the
lg/Lie-exports
branch
from
b626ba8
to
f165a61
Compare
| @@ -3,11 +3,11 @@ | |||
| # more efficient implementation. | |||
|
|
|||
| function combinations(n::Integer, k::Integer) | |||
| return sort(subsets(n, k)) | |||
| return sort(AbstractAlgebra.combinations(n, k)) | |||
There was a problem hiding this comment.
That's an unrelated change? (Not that I terribly mind, just am surprised)
There was a problem hiding this comment.
When fixing the calls to this method outside of Oscar.LieAlgebras I found this as an alternative. It does not make any speed difference in the limited places it is called anyway. So this change is more a note to me to see if I can get rid of Oscar.LieAlgebras.combinations in the near future.
| @@ -1333,6 +1333,7 @@ export ring_elem_type | |||
| export ring_type | |||
| export rising_factorial | |||
| export root | |||
| export roots | |||
There was a problem hiding this comment.
again, I am not really objecting, just surprised as this seems to go beyond the PR description, so I am double checking to verify this is intended?
There was a problem hiding this comment.
roots is reexported from Hecke anyway. Oscar.LieAlgebras was kind of inconsistent about exporting it.
And it felt somehow wrong to export it from Oscar in the LieAlgebras experimental folder, although it is not introduced there (as everything else that is exported there) but imported from Oscar/Hecke/...
This felt cleaner than just removing the export from OSCAR entirely and rely on the reexport
lgoettgens
force-pushed
the
lg/Lie-exports
branch
from
e74d7db
to
c14fee1
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4036 +/- ##
==========================================
+ Coverage 84.58% 84.79% +0.21%
==========================================
Files 597 599 +2
Lines 82237 84051 +1814
==========================================
+ Hits 69559 71275 +1716
- Misses 12678 12776 +98
|
lgoettgens
changed the title
Oscar.LieAlgebrasOscar.LieAlgebras
|
Bump @fingolfin |
|
Bump @fingolfin . It would be great to get this in before #4053 to avoid merge conflicts. |
|
Sorry I didn't see your "bump" here -- I regularly get flooded with GitHub notifications. I recommend pinging me on Slack again, that has a much better signal-to-noise ratio |
as suggested by @fingolfin in #4031 (comment).