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 printing of Term(identity, x) #450

Merged
merged 4 commits into from
Aug 23, 2022
Merged

Simplify printing of Term(identity, x) #450

merged 4 commits into from
Aug 23, 2022

Conversation

moble
Copy link
Contributor

@moble moble commented Jun 30, 2022

It can be helpful to encode a quantity q that should not be evaluated as Term(identity, q). This is the idea behind JuliaSymbolics/Symbolics.jl#638, which allows irrationals like π to be treated symbolically, rather than (typically) as Float64 in symbolic expressions. But that means that identity(π) appears everywhere we would normally just write π. This PR simply adds another case to show_term, to check if the function is identity, and just shows the argument instead.

@moble
Copy link
Contributor Author

moble commented Jul 28, 2022

@shashi @YingboMa Given that JuliaSymbolics/Symbolics.jl#638 has been merged, I think this is more relevant now. Any thoughts?

@shashi
Copy link
Member

shashi commented Jul 28, 2022

Ahh. Well I don't know how I feel about this haha. Can we take this branch only if args[1] is not issym(..) or istree(..) that way it should only do it for constants. At the very least I agree we should do it for Irrationals. @YingboMa

@YingboMa
Copy link
Member

YingboMa commented Aug 1, 2022

Yeah, I agree. We should check that args[1] is not symbolic.

src/types.jl Outdated Show resolved Hide resolved
@moble
Copy link
Contributor Author

moble commented Aug 1, 2022

@shashi Tests pass locally. I added the new test at the bottom of test/code.jl; let me know if you want it somewhere else.

@shashi
Copy link
Member

shashi commented Aug 2, 2022

You can just use repr to get a string... but looks good.

@shashi shashi merged commit dcbe861 into JuliaSymbolics:master Aug 23, 2022
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

Successfully merging this pull request may close these issues.

3 participants