Opened 10 years ago
Closed 8 years ago
#8908 closed enhancement (fixed)
Add the Young subgroup method to symmetric groups
Reported by: | jipilab | Owned by: | joyner |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.4 |
Component: | group theory | Keywords: | symmetric group |
Cc: | Merged in: | sage-5.4.beta2 | |
Authors: | Jean-Philippe Labbé | Reviewers: | Mike Hansen, Frédéric Chapoton |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
To every composition is associated a Young subgroup. This method returns the Young subgroup associated to the given composition.
Apply only trac_8908_young_subgroup_folded-2.patch
Attachments (1)
Change History (24)
comment:1 Changed 10 years ago by
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
Could you please
- fold the patches into one patch
- add #8908 to the first description line (instead of trac 8908)
It may then allow to have a green light from the bot.
comment:3 Changed 9 years ago by
I fixed the description line of the second patch. I hope that the bot will like it.
comment:4 Changed 9 years ago by
I would give a positive review if you could fold the two patches into one.
The bot is currently not working, so one does not require the green light.
comment:5 Changed 9 years ago by
- Status changed from needs_review to needs_work
comment:6 Changed 9 years ago by
- Status changed from needs_work to needs_review
I attached a folded patch. This one should work.
comment:7 Changed 9 years ago by
- Status changed from needs_review to positive_review
This is ok for me, assuming that all tests pass ; I have not checked that.
comment:8 Changed 9 years ago by
- Status changed from positive_review to needs_work
The last patch was tested on 4.7.2 and passed.
comment:9 Changed 9 years ago by
- Status changed from needs_work to needs_review
Changed to needs review: Test passes now.
comment:10 Changed 9 years ago by
For the patchbot :
Apply trac_8908_young_subgroup_folded.patch
comment:11 Changed 9 years ago by
- Reviewers set to Mike Hansen
Generally, I think this is good, but I changed it so that it works with the symmetric group on an arbitrary domain. Could you look over my changes?
comment:12 Changed 9 years ago by
For the patchbot :
Apply
trac_8908_young_subgroup_folded.patch trac_8908-young_subgroup-review.patch
comment:13 Changed 9 years ago by
Apply trac_8908_young_subgroup_folded.patch trac_8908-young_subgroup-review.patch
comment:14 Changed 8 years ago by
- Status changed from needs_review to needs_work
Well, this seems ok to me. I would rather replace
gen = self((domain[pos + i], domain[pos + i + 1])) gens.append(gen)
by the single line
gens.append(self((domain[pos + i], domain[pos + i + 1])))
I am a bit puzzled by the "PluginFailed?" given by the bot. What is the meaning of ValueError?("Mercurial queue boilerplate") ?
comment:15 Changed 8 years ago by
- Status changed from needs_work to needs_review
I've made that change.
Apply trac_8908_young_subgroup_folded.patch, trac_8908-young_subgroup-review.patch
comment:16 Changed 8 years ago by
- Description modified (diff)
comment:17 Changed 8 years ago by
- Milestone changed from sage-5.3 to sage-5.4
comment:18 Changed 8 years ago by
- Description modified (diff)
comment:19 Changed 8 years ago by
comment:20 Changed 8 years ago by
- Description modified (diff)
Apply only trac_8908_young_subgroup_folded-2.patch
Changed 8 years ago by
comment:21 Changed 8 years ago by
- Milestone changed from sage-5.4 to sage-5.3
comment:22 Changed 8 years ago by
- Keywords symmetric group added
- Reviewers changed from Mike Hansen to Mike Hansen, Frédéric Chapoton
- Status changed from needs_review to positive_review
applies on 5.4beta1, all tests pass, doc is ok: positive review
comment:23 Changed 8 years ago by
- Merged in set to sage-5.4.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
I uploaded the patch.
attachment:trac_8908-young_subgroup.patch