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 chapoton)

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)

trac_8908_young_subgroup_folded-2.patch (1.9 KB) - added by chapoton 8 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 Changed 10 years ago by jipilab

  • Authors set to Jean-Philippe Labbé
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by chapoton

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 jipilab

I fixed the description line of the second patch. I hope that the bot will like it.

comment:4 Changed 9 years ago by chapoton

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 chapoton

  • Status changed from needs_review to needs_work

comment:6 Changed 9 years ago by jipilab

  • Status changed from needs_work to needs_review

I attached a folded patch. This one should work.

comment:7 Changed 9 years ago by chapoton

  • 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 jipilab

  • 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 jipilab

  • Status changed from needs_work to needs_review

Changed to needs review: Test passes now.

comment:10 Changed 9 years ago by slabbe

For the patchbot :

Apply trac_8908_young_subgroup_folded.patch

comment:11 Changed 9 years ago by mhansen

  • 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 mhansen

For the patchbot :

Apply

trac_8908_young_subgroup_folded.patch trac_8908-young_subgroup-review.patch

comment:13 Changed 9 years ago by mhansen

Apply trac_8908_young_subgroup_folded.patch trac_8908-young_subgroup-review.patch

comment:14 Changed 8 years ago by chapoton

  • 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 mhansen

  • 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 chapoton

  • Description modified (diff)

comment:17 Changed 8 years ago by chapoton

  • Milestone changed from sage-5.3 to sage-5.4

comment:18 Changed 8 years ago by chapoton

  • Description modified (diff)

comment:20 Changed 8 years ago by chapoton

  • Description modified (diff)

Changed 8 years ago by chapoton

comment:21 Changed 8 years ago by chapoton

  • Milestone changed from sage-5.4 to sage-5.3

comment:22 Changed 8 years ago by chapoton

  • 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 jdemeyer

  • Merged in set to sage-5.4.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.