Opened 5 years ago

Closed 5 years ago

#23107 closed enhancement (fixed)

py3: fix everything in src/sage/matrix/*

Reported by: Salvatore Stella Owned by:
Priority: major Milestone: sage-8.0
Component: python3 Keywords:
Cc: Frédéric Chapoton Merged in:
Authors: Salvatore Stella Reviewers: Jeroen Demeyer, Travis Scrimshaw, Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: aa3319f (Commits, GitHub, GitLab) Commit: aa3319f7a7a92f3b5507f7d11f4a7c0d8ff0a49b
Dependencies: Stopgaps:

Status badges

Description

Part of #16081

This set of changes takes care of all the problems reported by sage -t --long src/sage/matrix/*. With probability 1 there are other functions that would need to be changed as well but that are not tested enough to raise errors at the moment.

It is my first ticket concerning pythn3 so please be extra careful when reviewing it.

This marginally changed also src/sage/graphs/generic_graph.py.

Change History (55)

comment:1 Changed 5 years ago by Salvatore Stella

Branch: public/ticket/23107
Commit: 91a7320801d289bd2a57e26ef14df0d56d696f84

Last 10 new commits:

0256751src/sage/matrix/matrix1.pyx
dc88750src/sage/matrix/matrix_double_dense.pyx
23fbdffsrc/sage/matrix/matrix_integer_sparse.pyx
fc2cc38src/sage/matrix/matrix_modn_sparse.pyx
a1c85e2src/sage/matrix/matrix_rational_sparse.pyx
9c6825bsrc/sage/matrix/matrix_sparse.pyx
60f1e0bsrc/sage/matrix/matrix_modn_dense_template.pxi
f2b607esrc/sage/graphs/generic_graph.py
b9075efsrc/sage/matrix/matrix_mod2_dense.pyx
91a7320src/sage/matrix/matrix_modn_dense_template.pxi

comment:2 Changed 5 years ago by Salvatore Stella

Status: newneeds_review

comment:3 Changed 5 years ago by Salvatore Stella

Type: PLEASE CHANGEenhancement

comment:4 Changed 5 years ago by git

Commit: 91a7320801d289bd2a57e26ef14df0d56d696f842108e1bac5d6a46a05b8165b39f96c1fbbc3ef4c

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

b286036src/sage/matrix/matrix1.pyx
b3d427fsrc/sage/matrix/matrix_double_dense.pyx
8c31595src/sage/matrix/matrix_integer_sparse.pyx
f96bfc6src/sage/matrix/matrix_modn_sparse.pyx
4521f1dsrc/sage/matrix/matrix_rational_sparse.pyx
970bde8src/sage/matrix/matrix_sparse.pyx
a015c6fsrc/sage/matrix/matrix_modn_dense_template.pxi
4e8346fsrc/sage/graphs/generic_graph.py
f51aaf6src/sage/matrix/matrix_mod2_dense.pyx
2108e1bsrc/sage/matrix/matrix_modn_dense_template.pxi

comment:5 Changed 5 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work
  1. I would replace
                from collections import Iterator, Sequence
                if isinstance(columns, (Iterator, Sequence)):
                    columns = list(columns)
                else:
                    raise TypeError("columns (=%s) must be a list of integers" % columns)
    

by

            columns = list(columns)
  1. Don't break doctests! You should keep the test A(range(4)), just change it to
    sage: from six.moves import range
    sage: A(range(4))
    

if you want to do the same thing in Python 2 and 3.

comment:6 Changed 5 years ago by git

Commit: 2108e1bac5d6a46a05b8165b39f96c1fbbc3ef4c869464dc611b299e8c28794470277d6ed399e903

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

869464dReintroduced test

comment:7 Changed 5 years ago by Salvatore Stella

Hi Jeroen, thanks for the fast reply.

  1. I bowed to your knowledge here and just reintroduced the test.
  1. The rationale behind the conditional clause you mention (and the many other similar occurrences I added in this partch) is that, most likely, columns is already going to be a list. I figured it was more efficient to add two checks rather than recreating a new list blindly. In this specific case you mention it will probably not matter because these lists will have short lengths anyway. The cases in which this is more relevant are probably in src/sage/matrix/matrix_integer_sparse.pyx where, in principle, one could be building a very big matrix. Should I remove these tests?

One other thing: before settling on this form I was just checking for hasattr(columns, "__iter__") but then I realized that also dictionaries will satisfy this.


New commits:

869464dReintroduced test

comment:8 in reply to:  7 Changed 5 years ago by Jeroen Demeyer

Replying to etn40ff:

  1. The rationale behind the conditional clause you mention (and the many other similar occurrences I added in this partch) is that, most likely, columns is already going to be a list.

This reply seems unrelated to the comment I made. It's not about calling list() when the input is not a list, it's about rejecting things which are not a Sequence or Iterator.

I'm perfectly fine with code like

if not isinstance(foo, list):
    foo = list(foo)

As you say, this is more efficient if the input is already a list.

comment:9 Changed 5 years ago by Jeroen Demeyer

Also note that isinstance(foo, list) is extremely fast in Cython since there is a particular optimizating in CPython for checking lists.

comment:10 Changed 5 years ago by Travis Scrimshaw

I agree with Jeroen, you want to construct a list and let Python fail when list(foo) doesn't work. We shouldn't force it to be either an Iterator or Sequence, and fail otherwise. This would allow things like the Python3 range to be valid input. Plus it could get rid of the (relatively) expensive from collections import Iterator, Sequence.

comment:11 Changed 5 years ago by Salvatore Stella

I am confused. I assumed we do not want to allow this to work if foo is a dictionary, do we?

comment:12 Changed 5 years ago by Travis Scrimshaw

Then we can explicitly forbid that, but I think this would break Python's ducktyping principle. Since it is suppose to take any iterable, and a dict is an iterable, it should work. A user passing a dict is likely an error on the user's part, but there could be a use case for it.

comment:13 Changed 5 years ago by Salvatore Stella

OK, I consider myself voted down and I will remove these checks.

I would only like to point out that we are changing the behaviour from the one before this patch.

The current patch:

-        if isinstance(columns, xrange):
-            columns = list(columns)
-        elif not isinstance(columns, (list, tuple)):
-            raise TypeError("columns (=%s) must be a list of integers" % columns)
+        if not isinstance(columns, (list, tuple)):
+            from collections import Iterator, Sequence
+            if isinstance(columns, (Iterator, Sequence)):
+                columns = list(columns)
+            else:
+                raise TypeError("columns (=%s) must be a list of integers" % columns)

The proposed version:

-        if isinstance(columns, xrange):
-            columns = list(columns)
-        elif not isinstance(columns, (list, tuple)):
-            raise TypeError("columns (=%s) must be a list of integers" % columns)
+        if not isinstance(columns, (list, tuple)):
+             columns = list(columns)

comment:14 in reply to:  11 Changed 5 years ago by Jeroen Demeyer

Replying to etn40ff:

I assumed we do not want to allow this to work if foo is a dictionary, do we?

Why should we explicitly disallow a dict?

comment:15 Changed 5 years ago by Salvatore Stella

Let me rephrase:

  1. With the code in sage before this ticket was opened the function in the comment 13 fails if you pass a dictionary to it (or any other iterable that is not xrange list or tuple).
  1. With the change I made it preserves this behaviour.
  1. With the change you suggest it works also if coulmns is a dictionary (using the its keys as elements).

I am not saying that 3 is bad (I actually prefer it), it is just different from 1.

comment:16 in reply to:  15 Changed 5 years ago by Jeroen Demeyer

Replying to etn40ff:

I am not saying that 3 is bad (I actually prefer it), it is just different from 1.

Well, you are already changing the behaviour of the function in other ways too.

If allowing dicts was the only change that you did, I would agree with you: that would be dubious. But if you change behaviour anyway, then the argument "it behaves different from the old code" makes little sense.

comment:17 Changed 5 years ago by Travis Scrimshaw

Something that is a problem with your current version is that it no longer would support xrange:

sage: from six.moves import range
sage: isinstance(range, Iterator)
False
sage: isinstance(range, Sequence)
False

comment:18 in reply to:  17 Changed 5 years ago by Salvatore Stella

Replying to tscrim:

Something that is a problem with your current version is that it no longer would support xrange:

sage: from six.moves import range
sage: isinstance(range, Iterator)
False
sage: isinstance(range, Sequence)
False

That is not surprising: you forgot to call it.

sage: from six.moves import range
sage: isinstance(range(10), Iterator)
False
sage: isinstance(range(10), Sequence)
True

Anyway this is a moot point now since we decided that I should remove these conditional statements.

comment:19 Changed 5 years ago by Travis Scrimshaw

*facepalm*

comment:20 Changed 5 years ago by Travis Scrimshaw

Reviewers: Jeroen Demeyer, Travis Scrimshaw

Can you revert some (all?) of the changes from range to explicit lists and be Python3 compatible? I think we should have coverage for (at least most) those as features. Once that is done, I think that takes care of all of the issues and we can set this to a positive review.

Also, author name.

comment:21 Changed 5 years ago by git

Commit: 869464dc611b299e8c28794470277d6ed399e9038a5b84949339869d42091328874bd8ac83ab45d7

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

e2749d7Merge branch 'public/ticket/23107' of git://trac.sagemath.org/sage into public/ticket/23107
5101129Revert to lists in src/sage/matrix/docs.py
c252f99Revert to lists in src/sage/matrix/matrix0.pyx
7c474a9Removed iterator tests from src/sage/matrix/matrix1.pyx
454671fRemoved iterator tests from src/sage/matrix/matrix1.pyx (bis)
6138023Revert to lists in src/sage/matrix/matrix_complex_ball_dense.pyx
0c6c2caRevert to lists in src/sage/matrix/matrix_integer_dense.pyx
af55f07Removed iterator test from src/sage/matrix/matrix_sparse.pyx
efa0b9dtypo
8a5b849Missing list

comment:22 Changed 5 years ago by Salvatore Stella

Authors: Salvatore Stella

Dear All, sorry for the late reply.

I made few changes to the code:

  1. All instances of [0,1] I introduced in the doctest are now list(range(2)). Same for [0,1,2,3]. I do not understand why you wanted this change (they produce the same output both in python2 and python3 and the fisrt syntax is shorter and more legible IMHO) but it is not worth spending time discussing this.
  1. I removed all the tests isinstance(foo, (Iterator, Sequence)) that where easily removeable. There are few more around, I think only in the following files
    src/sage/matrix/matrix_double_dense.pyx
    src/sage/matrix/matrix_integer_sparse.pyx
    src/sage/matrix/matrix_mod2_dense.pyx
    src/sage/matrix/matrix_modn_sparse.pyx
    src/sage/matrix/matrix_rational_sparse.pyx
    src/sage/matrix/matrix_space.py
    

The reason is that the function in there already accept a variety of inputs, some of which are not iterable, and to remove the tests these function would have to be significantly reworked. There are two solutions to this:

(cheap and messy) remove the test and fail if they are fed an iterator. This probably will require to fix doctests in other files. Plus I do not see why they should not work with iterators just because the implementation is not clean enough.

(more time consuming) rewrite the functions so that they default to evaluate foo = list(foo)

At the moment I do not have time to do so. If you are not satisfied feel free to take over this patch and fix it to your likings.

@Travis: I do not understand what you mean by "and be Python3 compatible". It looks to me that the changes this patch produces work in both version of python. This is the whole point, no?


New commits:

e2749d7Merge branch 'public/ticket/23107' of git://trac.sagemath.org/sage into public/ticket/23107
5101129Revert to lists in src/sage/matrix/docs.py
c252f99Revert to lists in src/sage/matrix/matrix0.pyx
7c474a9Removed iterator tests from src/sage/matrix/matrix1.pyx
454671fRemoved iterator tests from src/sage/matrix/matrix1.pyx (bis)
6138023Revert to lists in src/sage/matrix/matrix_complex_ball_dense.pyx
0c6c2caRevert to lists in src/sage/matrix/matrix_integer_dense.pyx
af55f07Removed iterator test from src/sage/matrix/matrix_sparse.pyx
efa0b9dtypo
8a5b849Missing list

New commits:

e2749d7Merge branch 'public/ticket/23107' of git://trac.sagemath.org/sage into public/ticket/23107
5101129Revert to lists in src/sage/matrix/docs.py
c252f99Revert to lists in src/sage/matrix/matrix0.pyx
7c474a9Removed iterator tests from src/sage/matrix/matrix1.pyx
454671fRemoved iterator tests from src/sage/matrix/matrix1.pyx (bis)
6138023Revert to lists in src/sage/matrix/matrix_complex_ball_dense.pyx
0c6c2caRevert to lists in src/sage/matrix/matrix_integer_dense.pyx
af55f07Removed iterator test from src/sage/matrix/matrix_sparse.pyx
efa0b9dtypo
8a5b849Missing list

comment:23 Changed 5 years ago by Salvatore Stella

Status: needs_workneeds_review

comment:24 Changed 5 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

There is still a lot of list(range(...)) in the doctests.

comment:25 Changed 5 years ago by Salvatore Stella

I am totally confused here. The code contained many instances of things like M[range(2):] and I changed that to M[[0,1],:]. Travis complained in comment 20 and I reverted them to M[list(range(2)):] which is the way things have been done in other tickets (c.f. #22972 https://git.sagemath.org/sage.git/commitid=5c740530b0224aab62eaa8e4e49d0dfd9a19ec08 ). Now you complain for the exact opposite reason. Can we please reach an agreement on what should be done?

comment:26 Changed 5 years ago by Jeroen Demeyer

The solution is easy: if the original was A, Travis complains about B and I complain about C, just keep A.

comment:27 in reply to:  25 Changed 5 years ago by Jeroen Demeyer

Replying to etn40ff:

#22972

Wow, that's bad indeed. I would never have agreed with that.

comment:28 Changed 5 years ago by Salvatore Stella

A is not an option: slicing does not work with iterators and I am not adderssing this issue in this ticket.

comment:29 Changed 5 years ago by Travis Scrimshaw

What has happened is you have increased support with changes like this:

  • src/sage/matrix/matrix1.pyx

    diff --git a/src/sage/matrix/matrix1.pyx b/src/sage/matrix/matrix1.pyx
    index b9c35be..dfe74a9 100644
    a b cdef class Matrix(matrix0.Matrix): 
    16971697            [5 4]
    16981698            [0 7]
    16991699        """
    1700         if isinstance(columns, xrange):
     1700        if not isinstance(columns, (list, tuple)):
    17011701            columns = list(columns)
    1702         elif not isinstance(columns, (list, tuple)):
    1703             raise TypeError("columns (=%s) must be a list of integers" % columns)
     1702
    17041703        cdef Matrix A
    17051704        cdef Py_ssize_t ncols,k,r
    17061705

and you do not need to explicitly specify it as a list. By allowing an xrange (which is Cython-speak for Python3 range) input, you are Python3 compatible. So I don't fundamentally see my and Jeroen's objections as being different (actually, mine was more meant as a question at the time).

Edit: Misread the code, it is not a regression but an expansion of support.

Last edited 5 years ago by Travis Scrimshaw (previous) (diff)

comment:30 in reply to:  28 ; Changed 5 years ago by Jeroen Demeyer

Replying to etn40ff:

A is not an option: slicing does not work with iterators and I am not adderssing this issue in this ticket.

Sorry, I don't understand what you mean. Why cannot you just keep the doctests as they are?

comment:31 Changed 5 years ago by Jeroen Demeyer

There is more general idea here: do not break doctests. If you make changes to the code which break a doctest, that's bad: you should fix your code such that the doctest remains to work.

There are always exceptions to this rule, but you need good reasons.

comment:32 in reply to:  30 Changed 5 years ago by Salvatore Stella

Replying to jdemeyer:

Replying to etn40ff:

A is not an option: slicing does not work with iterators and I am not adderssing this issue in this ticket.

Sorry, I don't understand what you mean. Why cannot you just keep the doctests as they are?

Because of this:

sage: M = Matrix(5); M[iter(range(3)):]
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-ca1bbd50e7c9> in <module>()
----> 1 M = Matrix(Integer(5)); M[iter(range(Integer(3))):]

/opt/sage/src/sage/matrix/matrix0.pyx in sage.matrix.matrix0.Matrix.__getitem__ (build/cythonized/sage/matrix/matrix0.c:7659)()
    984             r = self.matrix_from_rows(row_list)
    985         elif isinstance(row_index, slice):
--> 986             row_list = list(xrange(*row_index.indices(nrows)))
    987             r = self.matrix_from_rows(row_list)
    988         else:

TypeError: slice indices must be integers or None or have an __index__ method

comment:33 in reply to:  31 ; Changed 5 years ago by Salvatore Stella

Replying to jdemeyer:

There is more general idea here: do not break doctests. If you make changes to the code which break a doctest, that's bad: you should fix your code such that the doctest remains to work.

There are always exceptions to this rule, but you need good reasons.

I *did not* break a doctest. I have been applying the same procedure taht all the other python3 tickets did i.e. adapt the doctest to accept both python2 and python3. I agree that list(range( is fugly but that is what has been done all over the place.

comment:34 in reply to:  33 Changed 5 years ago by Jeroen Demeyer

Replying to etn40ff:

i.e. adapt the doctest to accept both python2 and python3

Adapting doctests is not a solution. The change from Python 2 to Python 3 should be as transparent as possible to the user. You cannot require the user to change all range(n) into list(range(n)).

(edit: as usual, there are exceptions. Some things cannot work in Python 3 for fundamental reasons. But I see no reason to make an exception for this ticket)

Last edited 5 years ago by Jeroen Demeyer (previous) (diff)

comment:35 Changed 5 years ago by Travis Scrimshaw

There is a difference between code that is designed to work with both types of range and those that currently need list(range(. Although perhaps on earlier Python3 tickets, Frédéric and I didn't quite know what the best way forward would be (such as Cython's support for xrange).

comment:36 in reply to:  35 ; Changed 5 years ago by Jeroen Demeyer

Replying to tscrim:

There is a difference between code that is designed to work with both types of range and those that currently need list(range(.

As I explained in 34, the solution is to change the code to accept range(n) in Python 3. Changing doctests is not a solution.

comment:37 in reply to:  36 ; Changed 5 years ago by Salvatore Stella

Cc: Frédéric Chapoton added

Replying to jdemeyer:

Replying to tscrim:

There is a difference between code that is designed to work with both types of range and those that currently need list(range(.

As I explained in 34, the solution is to change the code to accept range(n) in Python 3. Changing doctests is not a solution.

This patch does not address the issue of slicing; are you saying that it should leave alone the doctests failing because of this?

In any case, it looks to me that there is no consensus on what to do. Moreover we are not being consistent with other py3 tickets so far. Do we have to revisit all the work that has already been done? Frédéric should probably be part of the conversation as well so I just cced him.

I guess that me trying to simultaneously satisfy contrasting guidelines is pointless so I will step back and let you figure out a list of desiderata first. I do not think I will have much time to work on this in the foreseeable future (I am moving to another country for my next job) so please feel free to claim ownership of this ticket and continue without me.

comment:38 in reply to:  37 Changed 5 years ago by Jeroen Demeyer

Replying to etn40ff:

are you saying that it should leave alone the doctests failing because of this?

Exactly (to be clear, I assume that by "failing" you mean "failing on Python 3 but succeeding on Python 2").

Moreover we are not being consistent with other py3 tickets so far.

That's a pity indeed. Note that most of the Python 3 tickets do not change doctests that way. #22972 is the only one that I am aware of (and to repeat, I disagree with what was done there).

Do we have to revisit all the work that has already been done?

Well, in any case that's not really urgent. But at least let's not make things worse.

comment:39 Changed 5 years ago by Frédéric Chapoton

Branch: public/ticket/23107public/23107-v2
Commit: 8a5b84949339869d42091328874bd8ac83ab45d754645bb8c4320a2a0b685771c3e4e69582cde286

here is a modified branch..


New commits:

54645bbsome work on range in matrix folder

comment:40 Changed 5 years ago by Jeroen Demeyer

Just to remind that this is still needs_work.

comment:41 Changed 5 years ago by Frédéric Chapoton

Status: needs_workneeds_review

Thanks. I think Salvatore has dropped the case. I would have liked to see this resolved without me doing anything, but alas..

I am not totally convinced by the use of Iterator, Sequence.

comment:42 Changed 5 years ago by Salvatore Stella

Dear all, I just wanted to say that I did not drop the cause yet. I recently moved to my new job and I am still on a crazy rollercoaster trying to sort things out. If you are not in a hurry I can be back on this in a month time at the latest. On the other hand, if you are in a hurry feel free to push through without my input. There are going to be plenty of other py3 tickets I can contribute to once I am back.

Sorry for not being more helpful right now.

comment:43 Changed 5 years ago by Salvatore Stella

PS: how is it possible that I am listed as the one making the latest commit on the current branch when I did not?

comment:44 Changed 5 years ago by Frédéric Chapoton

Hello Salvatore,

in fact, there is no hurry (py3 is still a long way ahead, sadly), but I was afraid that you were (besides being very busy with installation at your new job, which is very understandable) somewhat horrified by the complexity of the matter, and the various contradictory requirements.

Concerning your name on the commit, I did a rebase, keeping only your first commit and squashing the others into this one, then changing its commit message. Then I worked a little bit and rebased again. So I guess this is considered to some kind of "transformation" of your initial commit.

comment:45 Changed 5 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work
  1. Why was this doctest removed:
    sage: A.delete_columns('junk')
    
  1. This might be partially a style issue, but I hate code of the form
    try:
        something()
    except TypeError:
        raise TypeError(...)
    

because usually, the exception that is caught and thrown away contains more useful information that the newly raised exception. So I would replace this by

something()

comment:46 Changed 5 years ago by Jeroen Demeyer

  1. For efficiency, it is better to reverse these conditions (replace A and B by B and A):
            if (isinstance(x, (Iterator, Sequence)) and
                not isinstance(x, (list, tuple))):
    

First of all, it's much cheaper to check if something is a list or tuple rather than some ABC type. Second, in many cases, the input will be a list or tuple, so then you don't even need the (Iterator, Sequence) check.

  1. Again for efficiency, move all imports of Iterator, Sequence to the top level, not inside the function. Since these are standard Python types, there should be no harm.

comment:47 Changed 5 years ago by Jeroen Demeyer

  1. There is still a list(range(3)) in src/sage/matrix/matrix_modn_dense_template.pxi

comment:48 Changed 5 years ago by Jeroen Demeyer

  1. In src/sage/matrix/matrix_modn_sparse.pyx, there is no reason to raise an error for input which is not Iterator, Sequence. Just convert to list as you do in some other places.

comment:49 Changed 5 years ago by git

Commit: 54645bb8c4320a2a0b685771c3e4e69582cde286a4bce3b1dae3d961a5ed301fa37e287f0195a745

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

39b799aMerge branch 'public/23107-v2' in 8.0.b12
a4bce3btrac 2310è some work on reviewer's comments

comment:50 Changed 5 years ago by git

Commit: a4bce3b1dae3d961a5ed301fa37e287f0195a745e2c758798bf8a5d873f9b3b6020706ed55f81e0d

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

e2c7587trac 23107 fixing things

comment:51 Changed 5 years ago by Frédéric Chapoton

Status: needs_workneeds_review

I think I have taken all of your comments into account. Back to needs review.

comment:52 Changed 5 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

In parent_old.pyx, better give up on trying to list all possible exceptions. If you replace that by except Exception, you can set this to positive review.

comment:53 Changed 5 years ago by git

Commit: e2c758798bf8a5d873f9b3b6020706ed55f81e0daa3319f7a7a92f3b5507f7d11f4a7c0d8ff0a49b

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

aa3319fReplace list with generic Exception catch in old parents.

comment:54 Changed 5 years ago by Travis Scrimshaw

Reviewers: Jeroen Demeyer, Travis ScrimshawJeroen Demeyer, Travis Scrimshaw, Frédéric Chapoton
Status: needs_workpositive_review

I took care of it.

comment:55 Changed 5 years ago by Volker Braun

Branch: public/23107-v2aa3319f7a7a92f3b5507f7d11f4a7c0d8ff0a49b
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.