glue-viz / glue-wwt

WorldWideTelescope viewer in glue
BSD 3-Clause "New" or "Revised" License
2 stars 6 forks source link

Truncate SkyCoord exception if too long #92

Closed Carifio24 closed 9 months ago

Carifio24 commented 1 year ago

This PR looks to solve a problem where the exception message can be very long when one of the latitude angles is not with [-90°, 90°]. This error message comes from astropy (see https://github.com/astropy/astropy/issues/13994), so this PR just truncates the message if it's longer than 300 characters (adding a ... to the end so the user knows that this isn't the full exception).

codecov[bot] commented 1 year ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (2c5b962) 71.55% compared to head (075263f) 69.21%. Report is 28 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #92 +/- ## ========================================== - Coverage 71.55% 69.21% -2.35% ========================================== Files 18 18 Lines 872 1020 +148 ========================================== + Hits 624 706 +82 - Misses 248 314 +66 ``` | [Files](https://app.codecov.io/gh/glue-viz/glue-wwt/pull/92?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz) | Coverage Δ | | |---|---|---| | [glue\_wwt/viewer/table\_layer.py](https://app.codecov.io/gh/glue-viz/glue-wwt/pull/92?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZV93d3Qvdmlld2VyL3RhYmxlX2xheWVyLnB5) | `78.40% <100.00%> (+1.66%)` | :arrow_up: | | [glue\_wwt/viewer/tests/test\_utils.py](https://app.codecov.io/gh/glue-viz/glue-wwt/pull/92?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZV93d3Qvdmlld2VyL3Rlc3RzL3Rlc3RfdXRpbHMucHk=) | `100.00% <100.00%> (ø)` | | | [glue\_wwt/viewer/tests/test\_wwt\_widget.py](https://app.codecov.io/gh/glue-viz/glue-wwt/pull/92?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZV93d3Qvdmlld2VyL3Rlc3RzL3Rlc3Rfd3d0X3dpZGdldC5weQ==) | `97.85% <100.00%> (+0.37%)` | :arrow_up: | ... and [4 files with indirect coverage changes](https://app.codecov.io/gh/glue-viz/glue-wwt/pull/92/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz)

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

Carifio24 commented 1 year ago

@dhomeier I've updated the message format here to give essentially the same method as your proposed upstream fix.

Carifio24 commented 1 year ago

Was just looking through some of my old open PRs. @astrofrog @dhomeier do we think the format of this message is good now? It matches the proposed upstream change quite closely.

Carifio24 commented 9 months ago

@astrofrog @dhomeier Since the issue this is meant to solve popped up again at Cape Code, I just wanted to check in on this and see whether we were happy with this format. I've added a couple tests to verify that the layer disabled message is what we expect to see.