Skip to content

Conversation

@mmatera
Copy link
Contributor

@mmatera mmatera commented Oct 17, 2020

Add support to compare SympyConstants

Copy link
Contributor

@weakit weakit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be wrong, but there seem to be a few unrelated changes in a few files. And a removed picture in the documentation.
Could you go over them?

It'd also be helpful if you could fix the tests and add a few new ones. I'll take a look at this tommo and maybe try to help.

graphics, graphics3d,
image, inout, integer, iohooks, linalg, lists, logic,
# image,
inout, integer, iohooks, linalg, lists, logic,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This right here is what I'm talking about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my mistake. I always comment out this module for local testings...

Copy link
Contributor

@weakit weakit Oct 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No probs, nobody's perfect!
I'm assuming the removed picture is also not intentional; add that back too?

@GarkGarcia GarkGarcia changed the title support for comparisons between SympyConstants Support for comparisons between SympyConstants Oct 18, 2020
@rocky rocky force-pushed the master branch 7 times, most recently from ab12a77 to 6525df7 Compare October 24, 2020 16:36
@weakit
Copy link
Contributor

weakit commented Nov 5, 2020

This wouldn't really work for something like Max[Pi, Pi + 1], would it?

@mmatera
Copy link
Contributor Author

mmatera commented Nov 5, 2020

This wouldn't really work for something like Max[Pi, Pi + 1], would it?

No as it is. This PR requires more work, but I didn't find the time to do it. The idea was first to identify where in the code the comparison is done, and then, look for the smartest way to extend it using sympy.

@weakit
Copy link
Contributor

weakit commented Nov 5, 2020

This PR requires more work, but I didn't find the time to do it.

I think I'll try working on it later today.

@rocky rocky force-pushed the master branch 3 times, most recently from 7a4d68a to d41fae8 Compare November 14, 2020 18:16
@rocky
Copy link
Member

rocky commented Nov 14, 2020

@mmatera is this still relevant?

@weakit
Copy link
Contributor

weakit commented Nov 15, 2020

I don't think so. PR #1000 closed #965.

@mmatera mmatera closed this Nov 15, 2020
@mmatera mmatera deleted the comparisons-sympyconstants branch January 6, 2021 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants