Opened 6 years ago

Closed 6 years ago

#15391 closed enhancement (fixed)

Implementing the Foata bijection on permutations

Reported by: MariaMonks Owned by:
Priority: major Milestone: sage-6.1
Component: combinatorics Keywords: days54, combinat, findstat02
Cc: jessicapalencia, tscrim, sage-combinat Merged in:
Authors: Maria Monks Reviewers: Jessica Striker, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: public/combinat/permutation/foata-15391 (Commits) Commit: 5a60319ced9c1921892f2b4235a4439e5a84672e
Dependencies: Stopgaps:

Description (last modified by darij)

Adds a method for applying the Foata bijection to a permutation.

Change History (19)

comment:1 Changed 6 years ago by MariaMonks

  • Branch set to public/combinat/permutation/foata-15391
  • Cc tscrim sage-combinat added
  • Commit set to b2ba47e743d882999d716f4ac810916eda534413
  • Status changed from new to needs_review

New commits:

b2ba47eAdded Foata bijection

comment:2 Changed 6 years ago by MariaMonks

  • Keywords findstat02 added

comment:3 Changed 6 years ago by git

  • Commit changed from b2ba47e743d882999d716f4ac810916eda534413 to be7baf7a74abaa63efb165a429d3699535aebb43

Branch pushed to git repo; I updated commit sha1. New commits:

be7baf7Doc test edits

comment:4 Changed 6 years ago by jessicapalencia

  • Reviewers set to Jessica Striker

Hi Maria, The code looks good. A couple of the doc tests failed, though. After you edit the permutation.py file, you can run the doc tests by typing from the command line: sage -bt permutation.py

(the -b builds sage and the -t runs the doc tests in the file you specify)

This rebuilds sage with your code inserted and runs the doc tests. I fixed the syntax on your doc tests so that they work and pushed the changes to trac. You can see the diff by clicking on (commits) in the branch field above. Or you can pull the changes to your local machine to test them out. If you are happy with my changes, you may set the ticket to positive review.

comment:5 Changed 6 years ago by git

  • Commit changed from be7baf7a74abaa63efb165a429d3699535aebb43 to 6906e12fa9f32b19d3f8e45e82cc2d7270972a27

Branch pushed to git repo; I updated commit sha1. New commits:

6906e12formatting changes (we aren't saving the environment)

comment:6 Changed 6 years ago by git

  • Commit changed from 6906e12fa9f32b19d3f8e45e82cc2d7270972a27 to 76b82a84d76a340d242cf4e6791dc763938f8d51

Branch pushed to git repo; I updated commit sha1. New commits:

76b82a8+reference +example
d27a8d8no need to compare e with itself

comment:7 Changed 6 years ago by darij

Hi Maria,

nice work. If you're fine with my changes, set this to positive_review. Unfortunately, a (trivial, I hope) conflict resolution might become necessary once my #15174 is merged into Sage.

Greets,
Darij

comment:8 Changed 6 years ago by git

  • Commit changed from 76b82a84d76a340d242cf4e6791dc763938f8d51 to 5a60319ced9c1921892f2b4235a4439e5a84672e

Branch pushed to git repo; I updated commit sha1. New commits:

5a60319Fixes to documentation.
c0fc6f4Some formatting changes.
1c1ee9fMerge branch 'master' into public/combinat/permutation/foata-15391

comment:9 Changed 6 years ago by tscrim

  • Reviewers changed from Jessica Striker to Jessica Striker, Travis Scrimshaw

I've updated it to the latest 5.13.beta3, simplified the logic and made some additional tweaks. I'm happy with it, so it's up to someone else to check my changes and set a positive review.

comment:10 Changed 6 years ago by MariaMonks

Hi all,

Sorry, I'm new to this. How do I see the changed file all in one place? Do I pull something from the repository? I just want to check it over before setting it to positive review. Thanks for all your tweaks!

Maria

comment:11 Changed 6 years ago by jessicapalencia

Hi Maria,

I was just looking at it now too. Yes, you should switch to your local branch and pull the changes from the trac server. I believe the commands are:

git fetch origin

pull --ff-only

Jessica

Last edited 6 years ago by jessicapalencia (previous) (diff)

comment:12 follow-up: Changed 6 years ago by jessicapalencia

After you pull, you can look at the new code, but if you want to test the features in Sage, you need to build sage:

sage -bt permutation.py

But for me, it's taking a long time to build, probably because it's upgrading to a new beta version. (Is this to be expected, Travis?)

Anyway, once you're happy with it, you can set it to positive review.

comment:13 in reply to: ↑ 12 Changed 6 years ago by tscrim

Replying to jessicapalencia:

But for me, it's taking a long time to build, probably because it's upgrading to a new beta version. (Is this to be expected, Travis?)

Yes, it's expected. The recent bump changes some low-level .pxi or similar such file that everything depends on, so a big recompile occurs. I got annoyed by this and merged the latest beta version into all my branches. Although this should improve significantly with #15430 (and ccache I think...) in the sense that we shouldn't need to do the recompile, or at least it will be significantly sped up, when changing between branches which change cython files.

comment:14 Changed 6 years ago by MariaMonks

  • Status changed from needs_review to positive_review

comment:15 Changed 6 years ago by MariaMonks

Ok, all the changes look good, and I set it to positive review. (And yes, I also had to leave sage to rebuild overnight.) What happens next?

comment:16 Changed 6 years ago by darij

  • Description modified (diff)
  • Milestone changed from sage-5.13 to sage-6.0

comment:17 Changed 6 years ago by darij

  • Description modified (diff)

In the case of no conflict, it gets merged into Sage 6.0 more or less automatically (unless the patchbot finds errors in the patch).

If some conflict arises (this might happen due to #15174 and other mercurial stuff that will probably get merged before this one), the release manager will reset this ticket to needs_work, and someone will have to make a merge commit to bring this patch up to date. "Someone" can be any of us, as resolving this kind of conflict (bugfixes in old methods vs. introduction of a new method) should be very easy. (Maybe the release manager will do it himself.)

Good work, Maria! (Already using the method...)

comment:18 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.0 to sage-6.1

comment:19 Changed 6 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.