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

Simplify on show is kinda annoying #15

Closed
MasonProtter opened this issue Feb 19, 2020 · 5 comments
Closed

Simplify on show is kinda annoying #15

MasonProtter opened this issue Feb 19, 2020 · 5 comments

Comments

@MasonProtter
Copy link
Member

Currently, if I write

julia> using SymbolicUtils, Test; @vars a b;

julia> a * b * a
((a ^ 2) * b)

julia> @test a * b * a == a^2 * b
Test Failed at REPL[33]:1
  Expression: a * b * a == a ^ 2 * b
   Evaluated: ((a ^ 2) * b) == ((a ^ 2) * b)
ERROR: There was an error during testing

Because we simplify the output of Base.show, errors testsets tricky to debug. notice it says Evaluated: ((a ^ 2) * b) == ((a ^ 2) * b)

One can use showraw to see what the actual representation is, but I think maybe we shouldn't be simplifying on show.

@shashi
Copy link
Member

shashi commented Feb 19, 2020

An alternative fix is to simplify both sides in ==. We kinda need some form of this to check equality of expressions (isequal compares structure, so we can keep it that way and have == compare the "value" (I forgot what the julia convention is though)).

scmutils simplifies on show, but this was kinda disappointing:

1 ]=> (= (+ 'x 1) (+ 'x 1))
#| #f |#

1 ]=> (eq? (+ 'x 1) (+ 'x 1))
#| #f |#

1 ]=> (eqv? (+ 'x 1) (+ 'x 1))
#| #f |#

1 ]=> (= (- (+ 'x 1) (+ 'x 1)) 0)
#| #f |#

1 ]=> (eq? (- (+ 'x 1) (+ 'x 1)) 0)
#| #f |#

1 ]=> (zero? (- (+ 'x 1) (+ 'x 1)))
#| #t |#

@shashi
Copy link
Member

shashi commented Feb 19, 2020

Haha I found this in my old code:

;;; Utility to compare equality of two expressions
(define (expr-eq? a b)
  (equal?
      (write-to-string (simplify a))
      (write-to-string (simplify b))))

@shashi
Copy link
Member

shashi commented Jun 19, 2020

Closing this since the printing is unsimplified in tests now.

@shashi shashi closed this as completed Jun 19, 2020
@MasonProtter
Copy link
Member Author

@shashi I feel that especially since this package is trying to be generic and more agnostic about the users' usecase, simplify in show is a bad idea.

I think that one major problem is that simplify makes interactively devising rules to manipulate a Term harder because the object's output at the REPL is not actually what you're trying to manipulate.


PS: sorry I haven't had much time for this package lately. I'm hoping to get back into it soon!

@shashi
Copy link
Member

shashi commented Jun 19, 2020

Ok sure, feel free to make the change by just changing the default value of simplified_show[]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants