Opened 6 years ago
Closed 6 years ago
#14783 closed enhancement (fixed)
Implement toggle group actions on posets
Reported by: | jessicapalencia | Owned by: | sage-combinat |
---|---|---|---|
Priority: | major | Milestone: | sage-5.12 |
Component: | combinatorics | Keywords: | poset, combinat, days49 |
Cc: | yanzhang, rowland, darij | Merged in: | sage-5.12.beta3 |
Authors: | Jessica Striker, Darij Grinberg | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #12848, #14267 | Stopgaps: |
Description (last modified by )
- Implement toggling a single elements in an order ideal
- Toggle a list of elements
- Implement row and column toggles
- Implement promotion and rowmotion
- Implement orbit structure calculator
Attachments (4)
Change History (22)
Changed 6 years ago by
comment:1 Changed 6 years ago by
- Status changed from new to needs_review
comment:2 Changed 6 years ago by
please add a commit message
comment:3 Changed 6 years ago by
- Description modified (diff)
comment:4 Changed 6 years ago by
Right, done.
comment:5 Changed 6 years ago by
for the bot:
apply trac_14783-toggles_on_order_ideals-jsdg.patch
comment:6 Changed 6 years ago by
Hey Darij,
A few more things:
- There's a "TODO" on line 371 of
finite_posets.py
, I believe that should be removed, element_constructor
ofrowmotion_orbits
is not documented,- "Returns" -> "Return",
- In
categories/posets.py
, could you make the`I`
's into``I``
and similar for`v`
and`vs`
(i.e. code formatting), - Could the
is_order_*
methods be moved into the categoryPosets
?
Thanks,
Travis
comment:7 Changed 6 years ago by
Good points, done! The diff for finite_posets.py is a bit weird, but it should work. I've also caught and fixed two wrong docstrings in sage/categories/posets.py.
The discrepancy between the lines 376 and 381 in finite_posets.py might itself be worth a ticket...
comment:8 Changed 6 years ago by
- Description modified (diff)
comment:9 Changed 6 years ago by
once again, your new patch needs a commit message, see the patchbot report
imho, having the patchbot green is a reasonable first step before starting the review process
comment:10 Changed 6 years ago by
Huh? Doesn't it say "trac #14783: implement toggle operations on order ideals of a poset"?
For patchbot:
apply trac_14783-toggles_on_order_ideals-jsdg.2.patch
comment:11 Changed 6 years ago by
yes indeed, but for the some reasons the bot managed to stack two patches that were not supposed to be stacked. And order-ideals.patch has no commit message..
comment:12 Changed 6 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
Looks good to me.
It's strange (and somewhat scary to me) the patchbot was able to apply both patches...
comment:13 Changed 6 years ago by
Thank you!
(I also don't see why the patchbot has been trying to do so in the first place...)
comment:14 Changed 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:15 Changed 6 years ago by
- Dependencies changed from #12848 to #12848, #14267
- Status changed from positive_review to needs_work
This needs to be rebased to #14267.
Changed 6 years ago by
comment:16 Changed 6 years ago by
- Status changed from needs_work to positive_review
Here trac_14783-toggles_on_order_ideals-jsdg-2.patch is the rebase. Sorry for the confusing filename; ignore all the other attachments.
patchbot:
apply trac_14783-toggles_on_order_ideals-jsdg-2.patch
comment:17 Changed 6 years ago by
- Description modified (diff)
comment:18 Changed 6 years ago by
- Merged in set to sage-5.12.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
today's work