Opened 8 years ago

Closed 7 years ago

#18075 closed enhancement (fixed)

Add inversion number method to AlternatingSignMatrices

Reported by: Jessica Striker Owned by: Jessica Striker
Priority: major Milestone: sage-6.6
Component: combinatorics Keywords: asm, days64, aim
Cc: Travis Scrimshaw, Kevin Dilks, Emily Gunawan Merged in:
Authors: Jessica Striker Reviewers: Darij Grinberg
Report Upstream: N/A Work issues:
Branch: 82c96d7 (Commits, GitHub, GitLab) Commit: 82c96d70c29144b8523ea48fbc4d44fb06dda9ca
Dependencies: Stopgaps:

Status badges

Description

Add inversion number method to AlternatingSignMatrices

Change History (18)

comment:1 Changed 8 years ago by Darij Grinberg

first!

comment:2 Changed 8 years ago by Jessica Striker

Branch: u/jessicapalencia/inversion_number-18075
Commit: 8827ffe72e10cce5491849558a45f919440a4ff6
Status: newneeds_review

New commits:

8827ffe18075: added inversion number to AlternatingSignMatrices

comment:3 Changed 8 years ago by Nicolas M. Thiéry

Just a couple suggestions:

  • Add the definition of the inversions number of an asm, typically with a reference
  • Explain that for permutation matrices this matches with the usual inversion number of the permutation, with an example where this is tested for all permutations of, say, 5.
  • Add a couple more examples
Last edited 8 years ago by Nicolas M. Thiéry (previous) (diff)

comment:4 Changed 8 years ago by Nicolas M. Thiéry

Thanks for your contribution to Sage :-)

comment:5 Changed 8 years ago by Nicolas M. Thiéry

Status: needs_reviewneeds_work
Work issues: more tests and doc

comment:6 Changed 8 years ago by git

Commit: 8827ffe72e10cce5491849558a45f919440a4ff6914e0ed6fbb1016c9b85300cc36dae0b2697492c

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

590afd1documentation and example added
914e0edAdded another test

comment:7 Changed 8 years ago by Jessica Striker

Status: needs_workneeds_review

comment:8 Changed 8 years ago by Darij Grinberg

Branch: u/jessicapalencia/inversion_number-18075public/ticket/18075
Commit: 914e0ed6fbb1016c9b85300cc36dae0b2697492ca524fad6890beff2421b7d1acd1965aa21d42379

New commits:

018f406Merge branch 'u/jessicapalencia/inversion_number-18075' of git://trac.sagemath.org/sage into asm
c6d341a#18075 review patch
a524fadticket 18075 review patch

comment:9 Changed 8 years ago by git

Commit: a524fad6890beff2421b7d1acd1965aa21d423790a442cb4befa9dbe0f5efa0df29d661dfbc7d788

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

0a442cbticket 18015 review patch

comment:10 Changed 8 years ago by Darij Grinberg

Work issues: more tests and docerror pointed out inside branch

Ignore my previous commits, and use the one below. Thanks, and sorry.


New commits:

0a442cbticket 18015 review patch

comment:11 Changed 8 years ago by git

Commit: 0a442cb4befa9dbe0f5efa0df29d661dfbc7d78882c96d70c29144b8523ea48fbc4d44fb06dda9ca

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

82c96d7correcting whitespace

comment:12 Changed 8 years ago by Darij Grinberg

Review branch uploaded.

A few remarks:

I am not 100% sure whether the letter ℓ goes against coding standards, but I have replaced it by a regular l to be safe. The docstrings are sent to latex, and I think latex doesn't like things that aren't ascii.

Also removed all the trailing whitespace.

The error in the iterator I pointed out should either be fixed here or moved to another ticket. Other than this, the ticket is good to go if you think the review changes are fine!

comment:13 Changed 8 years ago by Darij Grinberg

Reviewers: Darij Grinberg

comment:14 Changed 7 years ago by Jessica Striker

Hi Darij (or Travis),

Could you please open a new ticket for the error in the iterator or else fix the error on this ticket? I don't know exactly what the error is or how to fix it. Then can we set this ticket to positive review? Thanks!

comment:15 Changed 7 years ago by Darij Grinberg

Done, #18208. Can you pos_rev this one then?

Last edited 7 years ago by Darij Grinberg (previous) (diff)

comment:16 Changed 7 years ago by Jessica Striker

Status: needs_reviewpositive_review

Yes, thanks for reviewing it!

comment:17 Changed 7 years ago by Travis Scrimshaw

Work issues: error pointed out inside branch

comment:18 Changed 7 years ago by Volker Braun

Branch: public/ticket/1807582c96d70c29144b8523ea48fbc4d44fb06dda9ca
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.