Add in ordering='none' capability to srepr printer

[leerobert]

Brief description of what is fixed or changed

Currently the LatexPrinter and StrPrinter allow for ordering to be turned off (similar to evaluate=False in sympify. This capability however doesn't exist in the ReprPrinter.

StrPrinter(dict(order='none')).doprint(expr)

Now this is equivalent:

ReprPrinter(dict(order='none')).doprint(sympify('x + 3 - 2', evaluate=False)) == 'Add(Symbol('x'), Integer(3), Integer(-2))'

Other comments

Release Notes

• printing
  • allowed ReprPrinter to maintain original expr order via turning ordering off

[sympy-bot]

✅

Hi, I am the SymPy bot (v158). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

• printing
  • allowed ReprPrinter to maintain original expr order via turning ordering off (#18777 by @leerobert)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

#### Brief description of what is fixed or changed

Currently the `LatexPrinter` and `StrPrinter` allow for ordering to be turned off (similar to `evaluate=False` in sympify. This capability however doesn't exist in the `ReprPrinter`.

`StrPrinter(dict(order='none')).doprint(expr)`

Now this is equivalent:

`ReprPrinter(dict(order='none')).doprint(sympify('x + 3 - 2', evaluate=False)) == 'Add(Symbol('x'), Integer(3), Integer(-2))'`

#### Other comments


#### Release Notes

<!-- BEGIN RELEASE NOTES -->
* printing
  * allowed ReprPrinter to maintain original expr order via turning ordering off
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

[asmeurer]

This looks good. Actually I would rather expect srepr to show the true args ordering by default.

As an aside, I wonder why we don't just support order='none' in _as_ordered_terms directly.

[asmeurer]

Looks like tests are failing here.

[smichr]

Could we make this order == None? Do we have precedent for string vs None elsewhere?

[leerobert]

There are a few different ordering options that each printer supports (such as 'old' and 'lex') so that keeps in line with that convention. Having order=None means no ordering was provided rather than the "none" (aka don't reorder terms) ordering we are looking for. The overlap in terminology is confusing I do agree. The other printers use 'none' to denote no reordering of terms. We can revisit that terminology and sync the other printers accordingly but that would be backwards compatibility breaking and require to keep the old string lookup around.

[asmeurer]

Yeah, 'none' is already a pre-existing thing. It's pretty confusing I agree. IIRC None means use the default (which I think is 'lex') whereas 'none' means no ordering (all the orderings are strings).

[leerobert]

@asmeurer I should've left WIP in the title. I quickly put together this PR when I came across this issue on a project I'm working on. I'll take a look at the tests now.

I agree. I think we should allow for the order to be passed into Printer._as_ordered_terms. I'll refactor and fix tests.

[codecov]

Codecov Report

Merging #18777 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           master   #18777       +/-   ##
===========================================
- Coverage   75.66%   75.64%   -0.021%     
===========================================
  Files         647      647               
  Lines      168420   168433       +13     
  Branches    39682    39687        +5     
===========================================
- Hits       127428   127404       -24     
- Misses      35441    35473       +32     
- Partials     5551     5556        +5

[leerobert]

@asmeurer I'm worried about making srepr turn set ordering to none by default since that'll require a lot of changes to tests that currently use it. We could do that in a follow up PR to keep this one simple?

[asmeurer]

That's fine. My comment wasn't meant to mean that it needs to be done in this PR.