Repository: astropy/pyvo
Pull request #225 (2020-04-21 22:17:24)

Fixed formatter in IntervalQueryParam
#223 

codecov[bot]: # [Codecov](https://codecov.io/gh/astropy/pyvo/pull/225?src=pr&el=h1) Report
> Merging [#225](https://codecov.io/gh/astropy/pyvo/pull/225?src=pr&el=desc) into [master](https://codecov.io/gh/astropy/pyvo/commit/9d527e1caffaedf15a9b3580c31b26be8fff40e7&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/astropy/pyvo/pull/225/graphs/tree.svg?width=650&height=150&src=pr&token=Mynyo9xoPZ)](https://codecov.io/gh/astropy/pyvo/pull/225?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #225      +/-   ##
==========================================
+ Coverage   72.19%   72.20%   +0.01%     
==========================================
  Files          42       42              
  Lines        4478     4480       +2     
==========================================
+ Hits         3233     3235       +2     
  Misses       1245     1245              
```


| [Impacted Files](https://codecov.io/gh/astropy/pyvo/pull/225?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pyvo/dal/params.py](https://codecov.io/gh/astropy/pyvo/pull/225/diff?src=pr&el=tree#diff-cHl2by9kYWwvcGFyYW1zLnB5) | `86.40% <100.00%> (+0.10%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/astropy/pyvo/pull/225?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/astropy/pyvo/pull/225?src=pr&el=footer). Last update [9d527e1...4c97532](https://codecov.io/gh/astropy/pyvo/pull/225?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

funbaker: Is there a reason for a merge instead of a rebase?
andamian: > Is there a reason for a merge instead of a rebase?

Convenience. Is there an advantage for rebase? At the end we squash and merge to master.
funbaker: > > Is there a reason for a merge instead of a rebase?
> 
> Convenience. Is there an advantage for rebase? At the end we squash and merge to master.

https://www.atlassian.com/git/tutorials/merging-vs-rebasing

Patch:
diff --git a/pyvo/dal/params.py b/pyvo/dal/params.py
index 276f913..006a48d 100644
--- a/pyvo/dal/params.py
+++ b/pyvo/dal/params.py
@@ -380,7 +380,7 @@ class IntervalQueryParam(AbstractDalQueryParam):
         super().__init__()
 
     def get_dal_format(self, val):
-        if isinstance(val, tuple):
+        if isinstance(val, (tuple, Quantity)):
             if len(val) == 1:
                 high = low = val[0]
             elif len(val) == 2:
@@ -391,6 +391,10 @@ class IntervalQueryParam(AbstractDalQueryParam):
                                  format(val))
         else:
             high = low = val
+        if isinstance(low, (int, float)) and isinstance(high, (int, float))\
+                and low > high:
+            raise ValueError('Invalid interval: min({}) > max({})'.format(
+                low, high))
         if self._unit:
             if not isinstance(low, Quantity):
                 low = low*self._unit
@@ -398,9 +402,10 @@ class IntervalQueryParam(AbstractDalQueryParam):
             if not isinstance(high, Quantity):
                 high = high*self._unit
             high = high.to(self._unit, equivalencies=self._equivalencies).value
-        if low > high:
-            raise ValueError('Invalid interval: min({}) > max({})'.format(
-                low, high))
+
+            if low > high:
+                # interval could become invalid during transform (e.g. GHz->m)
+                low, high = high, low
 
         return '{} {}'.format(low, high)
 


Test patch:
diff --git a/pyvo/dal/tests/test_params.py b/pyvo/dal/tests/test_params.py
index 800237f..196ed74 100644
--- a/pyvo/dal/tests/test_params.py
+++ b/pyvo/dal/tests/test_params.py
@@ -8,7 +8,7 @@ from urllib.parse import parse_qsl
 
 from pyvo.dal.adhoc import DatalinkResults
 from pyvo.dal.params import find_param_by_keyword, get_converter,\
-    AbstractDalQueryParam
+    AbstractDalQueryParam, IntervalQueryParam
 from pyvo.dal.exceptions import DALServiceError
 
 import pytest
@@ -207,3 +207,18 @@ def test_dal_query_param():
     test_obs.clear()
     assert len(test_obs) == 0
     assert len(test_obs.dal) == 0
+
+
+def test_dal_format():
+    iqp = IntervalQueryParam(unit=u.m, equivalencies=u.spectral())
+    assert '1.0 1.0' == iqp.get_dal_format(1)
+    assert '1.0 2.0' == iqp.get_dal_format((1, 2))
+    assert '1.0 2.0' == iqp.get_dal_format((100*u.cm, 200*u.cm))
+    assert '1.0 2.0' == iqp.get_dal_format((100, 200)*u.cm)
+    assert '0.14989622900000002 1.0' == iqp.get_dal_format((100*u.cm, 2*u.GHz))
+    assert '14.9896229 29.9792458' == iqp.get_dal_format((0.01, 0.02)*u.GHz)
+    # Quantity intervals are corrected in terms of min and max ..
+    assert '1.0 2.0' == iqp.get_dal_format((2, 1)*u.m)
+    # But unitless intervals are not
+    with pytest.raises(ValueError):
+        iqp.get_dal_format((2, 1))