BUG: fix SkyView witdth and height input validation #2757
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2757 +/- ##
==========================================
+ Coverage 66.79% 66.81% +0.01%
==========================================
Files 237 237
Lines 18316 18316
==========================================
+ Hits 12234 12237 +3
+ Misses 6082 6079 -3 ☔ View full report in Codecov by Sentry. |
| size_deg = "{0},{1}".format(width.to(u.deg).value, | ||
| height.to(u.deg).value) | ||
| elif width and height: | ||
| elif width is not None and height is not None: |
There was a problem hiding this comment.
Shouldn't we instead check for Quantity inputs here? E.g. based on the docstring we are expecting Quantities or Nones, but not integers.
There was a problem hiding this comment.
the code allows float or integer arguments to be interpreted as degrees, which matches the skyview web api. But I agree, it would be better to require a qty explicitly
bsipocz
changed the title
|
I need to rebase this to retrigger CI (I see some testing issues when looking at this branch locally, but I'm fairly certain it's all unrelated to this PR and in fact have been fixed since this is open). |
My 1st PR, don't hesitate to feedback.
closes #2756