Update latexoperation.jl

co1emi11er2 commented 7 months ago

When interpolating Unitful types under an exponent, the latexoperation function should wrap ex.args[2] in parenthesis to properly display equation.

julia> using Unitful, UnitfulLatexify, Latexify
julia> x = 2u"inch"
julia> y = 2u"inch"
julia> @latexdefine z = $x^2 + $y^2
L"$z = 2\;\mathrm{inch}^{2} + 2\;\mathrm{inch}^{2} = 8\;\mathrm{inch}^{2}$"

298393262-bf7c74aa-9f3d-4e6f-83ee-83c5842ae1aa

You can see from the image above that it should look more like: z = (2 inch)^2 + (2 inch)^2 = 8 inch^2

This pull request fixes that issue. It checks whether the ex.args[2] is Real or is a Symbol, and if it isn't one of those, it wraps the ex.args[2] in parenthesis. I realize this may be too broad of a criteria. I didn't want to add any dependencies, but hopefully this pull request opens discussion on how best to fix this. Thanks!

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (6b869c3) 82.38% compared to head (4a28404) 82.38%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #280 +/- ## ======================================= Coverage 82.38% 82.38% ======================================= Files 21 21 Lines 829 829 ======================================= Hits 683 683 Misses 146 146 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

co1emi11er2 commented 7 months ago

Hi, I just wanted to bump this pull request and see if anyone had time to review this? I don't believe the failures are due to any changes I made. Thanks,

gustaphe commented 5 months ago

Hi! I'm sorry, I'm writing my thesis, not a lot of time and energy to go around. My feeling is this might indeed be too broad a distinction - though I can't think of any adverse examples right now. What should happen is that prevOp should be :* when an AbstractQuantity is latexified in UnitfulLatexify, but I don't think there's currently a method to communicate this from a latexrecipe. A tandem update of Latexify to include a Expr(:latexifymergeas, :*, ... and UnitfulLatexify to use it would probably be best, but maybe we could do with second best.

gustaphe commented 5 months ago

Actually, I can think of a couple of counterexamples. Arrays for one. Bra-kets is another (that we've used as an example of a latexifiable object before). I'm starting to think quantities are really the outliers in this respect - it would probably not be very good to have the bracketing per default.

co1emi11er2 commented 5 months ago

Yeah I thought of arrays as one that would now be wrapped in parenthesis with my update.

I can look into it more. My package Handcalcs.jl just does a lot more interpolation of symbols so with Unitful things just break under exponents. With my update everything works, but things like matrices and anyone elses types may just get that extra parenthesis. I think there is definitely a better solution though!

TS-CUBED commented 4 months ago

Are there any plans to include something like this in future versions of Latexify? I teach my students to use units in all their calculations, and this little issue trips them up a lot.

I currently use Cole's fix in a deved version of Latexify, but hope to convince other colleagues to move to the new system of creating learning material using Julia, for which this (deving modules) may be a bit too much of a hack.

gustaphe commented 4 months ago

Yes. I found a bit of time to work on this, #292 does it the way I think it should be done. It's a really simple change for UnitfulLatexify afterwards -- I already have it ready. Just going to give #292 a couple of days to fester.

gustaphe commented 3 months ago

This is now fixed, rejoice.

TS-CUBED commented 3 months ago

That's great. Will take it out of dev mode and update package.

–

Dr Torsten Schenkel Dr.-Ing. habil. CEng FIMechE

Associate Professor of Continuum Mechanics

Sheffield Hallam University Department of Engineering and Mathematics College of Business, Technology and Engineering Room 4206, Sheaf Building

Sheffield, S1 1WB UK

Tel +44 (0)114 225 6294 Profile: https://www.shu.ac.uk/about-us/our-people/staff-profiles/torsten-schenkel EC2M3 Lab: https://blogs.shu.ac.uk/ecm https://blogs.shu.ac.uk/ecm/ Github: https://github.com/TS-CUBED Github Pages: https://ts-cubed.github.io https://ts-cubed.github.io/

From: gustaphe @.> Sent: 03 July 2024 09:02 To: korsbo/Latexify.jl @.> Cc: Schenkel, Torsten @.>; Comment @.> Subject: Re: [korsbo/Latexify.jl] Update latexoperation.jl (PR #280)

This is now fixed, rejoice.

— Reply to this email directly, view it on GitHubhttps://github.com/korsbo/Latexify.jl/pull/280#issuecomment-2205353821, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMOMX2HCJZRI7HPWAOD7DDDZKOV2NAVCNFSM6AAAAABDDUIMLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBVGM2TGOBSGE. You are receiving this because you commented.Message ID: @.***>

co1emi11er2 commented 3 months ago

Thank you for the update! I have now updated my package for the changes as well!

korsbo / Latexify.jl

Update latexoperation.jl #280

Codecov Report