Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove old printing ordering at some point #4590

Closed
ryanGT opened this issue Jun 26, 2009 · 50 comments
Closed

Remove old printing ordering at some point #4590

ryanGT opened this issue Jun 26, 2009 · 50 comments
Assignees
Labels
Bug Deprecation Removal Tracks the removal of a deprecated feature. See github.com/sympy/sympy/wiki/Deprecating-policy imported printing
Milestone

Comments

@ryanGT
Copy link
Contributor

ryanGT commented Jun 26, 2009

Allows the output of _print_Add of the LatexPrinter to be sorted based on
the exponents of a main variable 'mainvar', which is an entry in the
self._settings dictionary.  'mainvar' can be a Symbol or a stirng.  If it
is a string, sympy.var is called.  There is also an option for 'descending'
in the dictionary that reverses the order of the arguments after they are
sorted.

The fix is in my github repo

git://github.com/ryanGT/sympy.git

on branch mainvar.

Ryan

Original issue for #4590: http://code.google.com/p/sympy/issues/detail?id=1491
Original author: https://code.google.com/u/110301006480078867972/
Original owner: https://code.google.com/u/101069955704897915480/

@ryanGT
Copy link
Contributor Author

ryanGT commented Jun 25, 2009

**Status:** NeedsReview  
**Owner:** ryanlists  
**Labels:** Latex  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c1
Original author: https://code.google.com/u/110301006480078867972/

@certik
Copy link
Member

certik commented Jun 25, 2009

**Labels:** NeedsReview Milestone-Release0.6.5  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c2
Original author: https://code.google.com/u/104039945248245758823/

@sympy-issue-migrator
Copy link

It seems that Wild('p2') is defined but not used. Apart from that it's a nice patch
and all tests run fine.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c3
Original author: https://code.google.com/u/Toon.Verstraelen@gmail.com/

@ryanGT
Copy link
Contributor Author

ryanGT commented Jul 4, 2009

Another good point and thanks for the review.  I removed Wild('p3') and replaced all
references to p3 with p2 (I thought it would be weird to have p1 and p3 without p2).

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c4
Original author: https://code.google.com/u/110301006480078867972/

@sympy-issue-migrator
Copy link

if I understood correctly, this is makes it possible to put an ordering on the printing.

Why not make it more general and let it accept a function (ordering) just as sorted
accepts a cmp method ( http://wiki.python.org/moin/HowTo/Sorting ) or a tuple so we can
specify an arbitrary lex order? (if this tuple only had 1 element it would be
equivalent to your code)

**Labels:** -Latex Printing  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c5
Original author: https://code.google.com/u/104906818019750310803/

@certik
Copy link
Member

certik commented Jul 8, 2009

The patch is in. What remains to be done is to implement Fabian's comment #5.

**Summary:** generalize mainvar in LatexPrinter  
**Labels:** -NeedsReview -Milestone-Release0.6.5  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c6
Original author: https://code.google.com/u/104039945248245758823/

@ryanGT
Copy link
Contributor Author

ryanGT commented Jul 8, 2009

More contorl over the ordering of _print_Add would be great.  This will probably be
my next contribution.  When I get ready to do that, I will look at the link from
Fabian's comment.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c7
Original author: https://code.google.com/u/110301006480078867972/

@ryanGT
Copy link
Contributor Author

ryanGT commented Jul 8, 2009

More contorl over the ordering of _print_Add would be great.  This will probably be
my next contribution.  When I get ready to do that, I will look at the link from
Fabian's comment.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c8
Original author: https://code.google.com/u/110301006480078867972/

@sympy-issue-migrator
Copy link

Ordering in _print_Add should not only be a part of the LatexPrinter. Can't we do
this in the Printer base class? One could implemented an iterator method like this:

class Printer(object):
    def iter_terms(self, expr):
        if not isinstance(expr, Add):
            raise TypeError()
        if self.add_cmp is None:
            for arg in expr.args:
                yield arg
        else:
            for arg in sorted(expr.args, cmp=self.add_cmp):
                yield arg


This iter method can be used by all printers who feel the need for it.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c9
Original author: https://code.google.com/u/Toon.Verstraelen@gmail.com/

@ryanGT
Copy link
Contributor Author

ryanGT commented Jul 17, 2009

Yes, something definitely needs to be done to easily allow all printers to ues the
same sorting functionality.  A Printer super-class seems logical.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c10
Original author: https://code.google.com/u/110301006480078867972/

@vks
Copy link
Contributor

vks commented Dec 6, 2009

**Summary:** implement ordering in printing  
**Status:** Accepted  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c11
Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

@mattpap
Copy link
Member

mattpap commented Apr 7, 2011

This issue has be fixed in full generality in polys12 branch, as a side effect of work on ordering of polynomials, e.g.:

In [1]: from sympy.printing.mathml import mathml

In [3]: f = x**2 + 2*x + 1

In [4]: print sstrrepr(f)
1 + 2*x + x**2

In [5]: print sstrrepr(f, order='lex')
x**2 + 2*x + 1

In [6]: print pretty(f)
           2
1 + 2⋅x + x 

In [7]: print pretty(f, order='lex')
 2          
x  + 2⋅x + 1

In [8]: print latex(f)
1 + 2 x + x^{2}

In [9]: print latex(f, order='lex')
x^{2} + 2 x + 1

In [10]: print mathml(f)
<apply><plus/><cn>1</cn><apply><times/><cn>2</cn><ci>x</ci></apply><apply><power/><ci>x</ci><cn>2</cn></apply></apply>

In [11]: print mathml(f, order='lex')
<apply><plus/><apply><power/><ci>x</ci><cn>2</cn></apply><apply><times/><cn>2</cn><ci>x</ci></apply><cn>1</cn></apply>

All orderings that Poly supports are available to printers. Ordering can be set in isympy via -o or --order. To make ordering neat, series are a special case:

$ bin/isympy -q -C -o lex
IPython console for SymPy 0.6.7-git (Python 2.6.6) (ground types: gmpy, cache: off)

In [1]: x**2 + 2*x + 1
Out[1]: 
 2          
x  + 2⋅x + 1

In [2]: series(sin(x), x, 0, 5)
Out[2]: 
     3          
    x           
x - ── + O(x**5)
    6

**Status:** Started  
**Owner:** matt...@gmail.com  
**Labels:** Milestone-Release0.7.0 NeedsReview  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c12
Original author: https://code.google.com/u/101069955704897915480/

@mattpap
Copy link
Member

mattpap commented Apr 29, 2011

All this is now in master. By default old ordering is used. If you want to use proper ordering, then start isympy with -o option or use 'order' keyword argument to printing functions or setup ordering in init_printing().

**Status:** Fixed  
**Labels:** -NeedsReview PassedReview  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c13
Original author: https://code.google.com/u/101069955704897915480/

@asmeurer
Copy link
Member

I think we should change the default order in isympy to lex.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c14
Original author: https://code.google.com/u/asmeurer@gmail.com/

@rlamy
Copy link
Member

rlamy commented Apr 29, 2011

I think the nice ordering ('lex') should be the default everywhere.

**Summary:** Use nice ordering in printing  
**Status:** Started  
**Labels:** -Milestone-Release0.7.0 -PassedReview  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c15
Original author: https://code.google.com/u/101272611947379421629/

@asmeurer
Copy link
Member

This should be easy to fix.  Just change the default and update the printer tests.

**Labels:** -Priority-Medium Priority-High Milestone-Release0.7.0 EasyToFix  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c16
Original author: https://code.google.com/u/asmeurer@gmail.com/

@mattpap
Copy link
Member

mattpap commented Apr 29, 2011

And all doctests.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c17
Original author: https://code.google.com/u/101069955704897915480/

@asmeurer
Copy link
Member

asmeurer commented May 9, 2011

I started looking at this, but it's unclear where the default should be set (there's bin/isympy, sympy/printing/printer.py, and sympy/interactive/printing.py).

Mateusz, you wrote the current init_printing() code.  What is the best way to do this?  I can make it use lex by default by changing the code in printing/printer.py, but then isympy -o old doesn't work correctly.  Part of the problem is the use of None for default values.  I could probably figure it out if I played around with it enough, but I figure that you would know how to do it right away since you already know that part of the code.

Also, see issue 5453 .

**Blockedon:** 5453  

Referenced issues: #5453
Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c18
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

asmeurer commented May 9, 2011

If you change the default ordering to lex, a couple of the test failures are outside of printing/, because they test the string of the expression.  These should be robustified, either by changing them to test the expression itself, or in some other way.

There are a lot of failures in test_evalf.py because we get (with lex ordering)

In [3]: 2 + 3*I
3⋅ⅈ + 2

and the tests test the string value of the output.  This is kind of a less than ideal order for numbers.  Is there a way to modify the way that lex works to make this print as 2 + 3*I without significantly redefining "lex" (it should be possible--just special case I)?

**Labels:** -EasyToFix  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c19
Original author: https://code.google.com/u/asmeurer@gmail.com/

@mattpap
Copy link
Member

mattpap commented May 9, 2011

I'm at a conference right now and till tomorrow I may not have internet connection, so I will reply to this and other issues tomorrow.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c20
Original author: https://code.google.com/u/101069955704897915480/

@asmeurer
Copy link
Member

asmeurer commented May 9, 2011

OK, thanks.  I am finally finished with classes, so I am going through all the release-0.7.0 issues and seeing what needs to be done.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c21
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

**Blocking:** 5487  

Referenced issues: #5487
Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c22
Original author: https://code.google.com/u/asmeurer@gmail.com/

@mattpap
Copy link
Member

mattpap commented May 20, 2011

Actually it was trivial to improve printing of complex numbers, see: https://github.com/mattpap/sympy/tree/complex-order

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c23
Original author: https://code.google.com/u/101069955704897915480/

@mattpap
Copy link
Member

mattpap commented May 20, 2011

I run tests and it doesn't look that bad. No more core test failures, only obvious ones in printing and code generation, and a few in simplify and ode (due to text-based test). Fixing doctests is a different story.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c24
Original author: https://code.google.com/u/101069955704897915480/

@mattpap
Copy link
Member

mattpap commented May 21, 2011

I fixed all test failures. The diff has less than 1000 lines and would be even shorter if some of the tests in printing weren't wrong (usually whitespace issues).

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c25
Original author: https://code.google.com/u/101069955704897915480/

@mattpap
Copy link
Member

mattpap commented May 21, 2011

I fixed bugs related to non-commutative symbols in ordering functions and improved ordering of certain classes of expressions when fixing doctests. Fixed all test, doctest and documentation failures. Now I have to clean up the branch and I will submit it soon.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c26
Original author: https://code.google.com/u/101069955704897915480/

@mattpap
Copy link
Member

mattpap commented May 21, 2011

Pull request is here: https://github.com/sympy/sympy/pull/344

**Labels:** -Priority-High Priority-Critical NeedsReview  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c27
Original author: https://code.google.com/u/101069955704897915480/

@rlamy
Copy link
Member

rlamy commented May 22, 2011

The pull request is in, but I think we should get rid of the old order completely.

**Labels:** -Priority-Critical -NeedsReview Priority-Medium  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c28
Original author: https://code.google.com/u/101272611947379421629/

@mattpap
Copy link
Member

mattpap commented May 22, 2011

Thanks. The old order will most certainly die naturally when Py3k port is ready, but I don't mind removing it earlier.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c29
Original author: https://code.google.com/u/101069955704897915480/

@asmeurer
Copy link
Member

Yeah, let's remove it eventually, but keep it around for now.

**Labels:** -Milestone-Release0.7.0 Milestone-Release0.8.0  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c30
Original author: https://code.google.com/u/asmeurer@gmail.com/

@mattpap
Copy link
Member

mattpap commented Oct 10, 2011

I'm pretty sure that no one will miss old order. 0.7.3 seems fine to me.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c37
Original author: https://code.google.com/u/101069955704897915480/

@vperic
Copy link
Contributor

vperic commented Oct 10, 2011

Could one of you officially deprecate it then, I don't know where it's all defined and I'd rather not find out. :)

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c38
Original author: https://code.google.com/u/108713607268198052411/

@vperic
Copy link
Contributor

vperic commented Oct 10, 2011

And the blocking is the other way around - we need this to be able to drop cmp_to_key. Fixing now.

**Blockedon:** -2674  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c39
Original author: https://code.google.com/u/108713607268198052411/

@vperic
Copy link
Contributor

vperic commented Oct 10, 2011

**Blocking:** 5773  

Referenced issues: #5773
Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c40
Original author: https://code.google.com/u/108713607268198052411/

@asmeurer
Copy link
Member

Bumping down the milestone to make sure we add a deprecation warning before the next release.

**Labels:** -Milestone-Release0.7.3 Milestone-Release0.7.2  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c41
Original author: https://code.google.com/u/asmeurer@gmail.com/

@vperic
Copy link
Contributor

vperic commented Oct 21, 2011

Hum, I just reread this and it occurs to me you only want to deprecate it for 0.7.3, while I wanted to deprecate it now and remove it by 0.7.3 (which will probably be about 6 months from now or so). I suppose it doesn't really matter, though.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c42
Original author: https://code.google.com/u/108713607268198052411/

@asmeurer
Copy link
Member

The point is that we never actually deprecated it with a warning.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c43
Original author: https://code.google.com/u/asmeurer@gmail.com/

@vperic
Copy link
Contributor

vperic commented Nov 4, 2011

Huh. I looked at this again but I'm really not sure where to put a warning exactly. It's not the easiest term to search for, either. Just point me in the right direction and I'll do it, though.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c44
Original author: https://code.google.com/u/108713607268198052411/

@asmeurer
Copy link
Member

asmeurer commented Nov 4, 2011

I guess the best place to put it would be Basic._compare_pretty.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c45
Original author: https://code.google.com/u/asmeurer@gmail.com/

@rlamy
Copy link
Member

rlamy commented Nov 4, 2011

Errr... There's already an @deprecated on Basic.compare_pretty. One warning is enough, surely?

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c46
Original author: https://code.google.com/u/101272611947379421629/

@asmeurer
Copy link
Member

asmeurer commented Nov 4, 2011

Ah, so apparently this was already done by 

commit aa94e72e5d53f0ae774cf114fae107ec0687bb21
Author: Mateusz Paprocki <mattpap@gmail.com>
Date:   Mon Oct 3 10:20:00 2011 -0700

    Don't use and deprecate Basic.compare_pretty()

    The only place where Basic.compare_pretty() is used in SymPy
    is in Printer where old term ordering is used. The default way
    to sort expressions is to use .sort_key() or default_sort_key.

So let's remove it after the next release.

**Labels:** -Milestone-Release0.7.2 Milestone-Release0.7.3  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c47
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

**Labels:** DeprecationRemoval  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c48
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

Postponing deprecation removals

**Labels:** -Milestone-Release0.7.3 Milestone-Release0.7.4  
**Blocking:** -sympy:2388 -sympy:2674 sympy:2388 sympy:2674  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c49
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

Postponing deprecation removals.

**Labels:** -Milestone-Release0.7.4 Milestone-Release0.7.5  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1491#c50
Original author: https://code.google.com/u/asmeurer@gmail.com/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Deprecation Removal Tracks the removal of a deprecated feature. See github.com/sympy/sympy/wiki/Deprecating-policy imported printing
Projects
None yet
Development

No branches or pull requests

8 participants