Skip to content

Conversation

@Ickaser
Copy link
Contributor

@Ickaser Ickaser commented Sep 5, 2025

As briefly discussed in #795 (comment), it's marginally more appealing to use a thin space (\,) than the wider space (\;) between the number and unit of a quantity. This is also the default behavior in LaTeX's siunitx.

This PR makes that change (fortunately a two-character change), as well as updating the corresponding tests.
Current master:
image

This PR:
image

The aspect which could depend on taste is that I also removed the spacing between an array and its unit, which seems to visually match the regular quantity spacing.

For arrays, options are:
With \, (third option this PR): (\begin{equation} \left[ \begin{array}{c} 1 \\ 2 \\ 3 \\ \end{array} \right]\,\mathrm{m} \end{equation})
image
With \; (current master):
image
With no extra spacing at all (this PR former version of this PR): (\begin{equation} \left[ \begin{array}{c} 1 \\ 2 \\ 3 \\ \end{array} \right]\mathrm{m} \end{equation})
image

The one caveat I see to removing the space between array and unit is that, formally, the space should be left out when the unit is degrees, and so this puts other units at the same separation as degrees (instead of having an extra space).

@codecov
Copy link

codecov bot commented Sep 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@d14ad5b). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #810   +/-   ##
=========================================
  Coverage          ?   89.46%           
=========================================
  Files             ?       21           
  Lines             ?     1699           
  Branches          ?        0           
=========================================
  Hits              ?     1520           
  Misses            ?      179           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@giordano
Copy link
Member

giordano commented Sep 8, 2025

Did you push cf9ad06 to the wrong branch?

@Ickaser
Copy link
Contributor Author

Ickaser commented Sep 8, 2025

Did you push cf9ad06 to the wrong branch?

Yep, reverting that here

@sostock
Copy link
Collaborator

sostock commented Sep 14, 2025

For consistency, I would go with the third option, but it’s not a strong preference.

@Ickaser
Copy link
Contributor Author

Ickaser commented Sep 15, 2025

Consistency is good, I'll put the \, back in. @gustaphe any thoughts on space between array and unit before this merges?

@gustaphe
Copy link
Contributor

No opinion, array times unit looks awkward either way, and when they are not side by side I doubt anyone will register the difference :D

I like the PR in general.

@sostock sostock merged commit 72dfb40 into JuliaPhysics:master Sep 16, 2025
14 checks passed
@Ickaser Ickaser deleted the quantity-spacing branch September 16, 2025 12:29
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.

4 participants