sympy_gamma icon indicating copy to clipboard operation
sympy_gamma copied to clipboard

Resolves & Fixes Issue #66

Open ashutoshsaboo opened this issue 8 years ago • 6 comments

Issue #66 resolved. Now, the equation listed in #66 - ((sin(x) - cos(x))(sin(x) + cos(x))) == (1 - 2 (cos(x)**2)) returns TRUE.

Same can be observed in the screenshot attached below-:

selection_010

@asmeurer Please review this PR.

Thanks! :smile:

ashutoshsaboo avatar Mar 17 '16 18:03 ashutoshsaboo

This doesn't seem like the right place in the code to implement this. It should be implemented as a card.

asmeurer avatar Mar 17 '16 18:03 asmeurer

@asmeurer Sorry, I didn't get your point. Any reason why? Because, it's just string manipulation, I am doing it here, since anywhere else in the cards, I am not able to get the user string input, and I get that only as parsed input(which is broken into different alphabets), and stored in some kind of alphabets.

That's the reason why I implemented it here. @asmeurer

ashutoshsaboo avatar Mar 18 '16 17:03 ashutoshsaboo

String manipulation should be done at the parser level. I think there might even already be some functions in the parsing module for detecting equalities.

asmeurer avatar Mar 18 '16 17:03 asmeurer

Ohh I see. I'll try to have a look at them @asmeurer .

BTW, Why do we generally categorize, like this needs to be done at the parser level? Is it just, so that the code remains organized, or is there any other reason? Since, I didn't know, so thought, I must ask.

Thanks :smile:

ashutoshsaboo avatar Mar 18 '16 17:03 ashutoshsaboo

Yes, it keeps the code organized. Also, the parser is a better abstraction for dealing with strings and converting them to SymPy input. Searching for substrings (like you are doing here) gets messy fast, and more importantly, won't always work in general, because you won't be able to handle things like matching parentheses or detecting when there is a string literal in the string itself. For instance, if someone wrote Symbol('=='), which is completely valid.

asmeurer avatar Mar 18 '16 17:03 asmeurer

@asmeurer Ohh yes I understand. I'll look into it, further deep, and try to push a new PR for this issue. Thanks for your input Sir. :smile:

ashutoshsaboo avatar Mar 18 '16 17:03 ashutoshsaboo