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

Some tests are broken for SymPy 1.4 #31

Open
utensil opened this issue Apr 12, 2019 · 5 comments
Labels
bug
Milestone

Comments

@utensil
Copy link
Member

@utensil utensil commented Apr 12, 2019

SymPy 1.4 has just released and some tests are broken for SymPy 1.4, see build https://circleci.com/workflow-run/dcbd3c98-45c3-4c89-abd6-2ea97fa07b2c .

Freeze SymPy version to 1.3 to keep CI green for now.

Tasks:

  • Fix broken collect() calls
  • The fix for order change issue also needs to be delayed until the explicitly change to use SymPy 1.4 in CI after the collect() issue is addressed
  • adding a CI build for SymPy master so these issues can be detected sooner
@utensil

This comment has been minimized.

Copy link
Member Author

@utensil utensil commented Apr 12, 2019

Errors are due to:

  • \quad replaced by \ for lists, tuples, and dicts, see better (smaller) spacing when generating LaTeX for lists, tuples, and dicts. (sympy/sympy#15970 by @asmeurer and @oscargus)
  • \left ( replaced by left(, also for \right and [{, see: LaTeX printing now consistently does not use a space after \left and \right (sympy/sympy#15779 by @oscargus)
  • some ordering has changed: eu.ev_r = -1/2 + 1/2 changed to eu.ev_r = 1/2 - 1/2, see: changed the print order of additions of form k - expr (used to print as -expr + k) (sympy/sympy#15985 by @nickovs)
  • all collect() call are broken, see: Raise exception when collecting a noncommutative symbol (sympy/sympy#15447 by @JMSS-Unknown)
@utensil utensil added the bug label Apr 12, 2019
@utensil utensil added this to the 0.4.4 milestone Apr 12, 2019
@oscargus

This comment has been minimized.

Copy link

@oscargus oscargus commented Apr 12, 2019

I'll see if I can update the printing tests for Galgebra correspondingly. (I did not find any instances of \quad in the tests files I found though.)

@utensil

This comment has been minimized.

Copy link
Member Author

@utensil utensil commented Apr 12, 2019

I did not find any instances of \quad in the tests files I found though.

That's because we are using nbval to verify LaTeX or enhanced console output for galgebra by the Jupyter Notebooks in examples/ipython. It's an effective way to write tests for a symbolic library that has multiple output formats, but it's also sensitive to subtle changes between versions.

I'll see if I can update the printing tests for Galgebra correspondingly.

Thanks!

The \quad and \left issue can be fixed by modifying https://github.com/pygae/galgebra/blob/15-print-pow/test/.nbval_sanitize.cfg to ignore this kind of differences.

The real difficulty is the collect() issue. Currently galgebra use collect() in quite some places to collect the bases of multivectors, probably should do it in another way, like manually collect the bases. It's correct for SymPy to raise exceptions if the symbols to be collected has no certain order in the expression, but in the case of the bases, they are always at the rightmost position so there's no confusion here.

The collect() issue must be fixed before we can change SymPy version in CI to 1.4. And It's non-trivial, so I'll not fix it in #17 but will fix it before 0.4.4 release.

The fix for order change issue also needs to be delayed until the explicitly change to use SymPy 1.4 in CI after the collect() issue is addressed.

@asmeurer

This comment has been minimized.

Copy link

@asmeurer asmeurer commented Sep 27, 2019

Once you get this fixed I would suggest adding a CI build for SymPy master so these issues can be detected sooner. We also plan to do a 1.5 release soon, so if you can test SymPy master now that would help.

@utensil

This comment has been minimized.

Copy link
Member Author

@utensil utensil commented Sep 27, 2019

Good idea, this was also brought up in sympy/sympy#8365 (comment) and I forgot it.

Added to the task list in the main thread of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.