erwanp / pytexit

Convert a Python expression to a LaTeX formula
https://pytexit.readthedocs.io/
Other
114 stars 31 forks source link

Feature/sympy simplify #24

Open alexhagen opened 3 years ago

alexhagen commented 3 years ago

Working simplification for Issue #22. Uses sympy to do the simplification.

Requested use case: py2tex('theta = w_k * a ** 4 / t ** 4 / E', simplify_fractions=True) now outputs $$\theta=\frac{a^{4} w_{k}}{E t^{4}}$$.

erwanp commented 3 years ago

There seems to be an index problem in the "sum ... until 20" test, which has been corrected to sum([... for i in range(20)]) although it is actually sum([... for i in range(21)]) in Python

alexhagen commented 3 years ago

Oh interesting. I'll check into that.

alexhagen commented 3 years ago

@erwanp I've fixed the index problem you pointed out, I think I must've forked from an old version. Let me know if anything else needs fixed.

erwanp commented 3 years ago

I think you can fix the last conflict from the online interface. Else I'll do it!

codecov[bot] commented 3 years ago

Codecov Report

Merging #24 (ae93b58) into master (b18e0f6) will decrease coverage by 1.67%. The diff coverage is 4.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
- Coverage   81.09%   79.41%   -1.68%     
==========================================
  Files          13       13              
  Lines         603      617      +14     
==========================================
+ Hits          489      490       +1     
- Misses        114      127      +13     
Impacted Files Coverage Δ
setup.py 0.00% <ø> (ø)
pytexit/core/core.py 87.17% <4.54%> (-3.41%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b18e0f6...ae93b58. Read the comment docs.

erwanp commented 3 years ago

@alexhagen

Test crash with :

I think we should maintain compatibility with 2.7 for some time, but this feature is nice. Can you move the import within the function maybe ?

alexhagen commented 3 years ago

For sure. Should I break this off from your simplify_fractions parameter, we could have another parameter use_sympy?

erwanp commented 3 years ago

I'd say keep it within simplify_fractions. #22 OP @turner-eng expected to find this behavior. Less parameters also make the code easier to understand. We could escape the simplify_fraction test with sys.version_info[0]