Add a (skippable) check that the axes have equal aspect ratio. by anntzer · Pull Request #61 · ppinard/matplotlib-scalebar (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation6 Commits2 Checks2 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

When the scalebar is not drawn on an aspect where imshow() has been called (e.g., a point cloud made with plot()), the axes aspect ratio is not automatically set to 1, in which case the scale bar will be wrong (or rather, it will only be correct in the direction (horizontal or vertical) in which it is drawn).

To avoid mistakes, add a check for the aspect ratio, emitting a warning when appropriate. The check can be skipped by using new variants for rotation: it can now be set to "horizontal-only" ("the scalebar only applies to the horizontal direction") or "vertical-only". (This could also have been a separate kwarg, but something like check_aspect=False reads a bit awkwardly to me -- let me know if you'd prefer that.)

Choose a reason for hiding this comment

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

colorbar? Shouldn't it be "scale bar"?

Choose a reason for hiding this comment

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

same here

Choose a reason for hiding this comment

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

I think the draw is important here to make sure the scale bar is drawn. It doesn't look that it will impact your unit test

Choose a reason for hiding this comment

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

Thank you for your contribution. I agree having a warning when the aspect ratio is not 1 would be useful, as 99% of the time the scale bar will be used on plots with an aspect ratio of 1.

When the scalebar is not drawn on an aspect where imshow() has been called (e.g., a point cloud made with plot()), the axes aspect ratio is not automatically set to 1, in which case the scale bar will be wrong (or rather, it will only be correct in the direction (horizontal or vertical) in which it is drawn).

To avoid mistakes, add a check for the aspect ratio, emitting a warning when appropriate. The check can be skipped by using new variants for rotation: it can now be set to "horizontal-only" ("the scalebar only applies to the horizontal direction") or "vertical-only". (This could also have been a separate kwarg, but something like check_aspect=False reads a bit awkwardly to me.)

Thanks, I addressed your comments.

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.93%. Comparing base (2c57173) to head (a30887f).
Report is 8 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@ Coverage Diff @@ ## master #61 +/- ##

• Coverage 91.46% 89.93% -1.54%

  Files 3 3
  Lines 422 437 +15

• Hits 386 393 +7

• Misses 36 44 +8

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

zezhong-zhang pushed a commit to zezhong-zhang/matplotlib-scalebar that referenced this pull request

Feb 20, 2025

…rd#61)

• Add a (skippable) check that the axes have equal aspect ratio.

When the scalebar is not drawn on an aspect where imshow() has been called (e.g., a point cloud made with plot()), the axes aspect ratio is not automatically set to 1, in which case the scale bar will be wrong (or rather, it will only be correct in the direction (horizontal or vertical) in which it is drawn).

To avoid mistakes, add a check for the aspect ratio, emitting a warning when appropriate. The check can be skipped by using new variants for rotation: it can now be set to "horizontal-only" ("the scalebar only applies to the horizontal direction") or "vertical-only". (This could also have been a separate kwarg, but something like check_aspect=False reads a bit awkwardly to me.)

• Try to fix CI

Co-authored-by: Philippe Pinard philippe.pinard@gmail.com

zezhong-zhang pushed a commit to zezhong-zhang/matplotlib-scalebar that referenced this pull request

Feb 20, 2025

…rd#61)

• Add a (skippable) check that the axes have equal aspect ratio.

When the scalebar is not drawn on an aspect where imshow() has been called (e.g., a point cloud made with plot()), the axes aspect ratio is not automatically set to 1, in which case the scale bar will be wrong (or rather, it will only be correct in the direction (horizontal or vertical) in which it is drawn).

To avoid mistakes, add a check for the aspect ratio, emitting a warning when appropriate. The check can be skipped by using new variants for rotation: it can now be set to "horizontal-only" ("the scalebar only applies to the horizontal direction") or "vertical-only". (This could also have been a separate kwarg, but something like check_aspect=False reads a bit awkwardly to me.)

• Try to fix CI

Co-authored-by: Philippe Pinard philippe.pinard@gmail.com