Repository: glotzerlab/signac

Issue:
Surprising type comparisons with $signac find
### Description

I have a statepoint parameter that has some `int` values and some `decimal` values. When I use `signac find` to find jobs filtering by this parameter, it only returns jobs where both the value and type of the jobs' statepoints match the search parameters. I would expect the behavior to match that of python's `==` operator, e.g., `$signac find x 10` would return jobs where `job.sp.x = 10` and `job.sp.x = 10.0`.

### To reproduce

```
$ signac init test
Initialized project 'test'.
$ python
>>> import signac
>>> project = signac.get_project()
>>> job = project.open_job({'x': 100**0.5})
>>> job.init()
>>> exit()
$ signac find x 10
Interpreted filter arguments as '{"x": 10}'.
$ signac find x 10.
Interpreted filter arguments as '{"x": 10.0}'.
13dd08001115e65c96a2eeda9ce3505b
```

### System configuration

Please complete the following information:

```
$ python -c 'import platform; print(platform.platform()); import sys; print(sys.version); import signac; print(signac.__version__)'
Darwin-17.7.0-x86_64-i386-64bit
3.5.5 | packaged by conda-forge | (default, Jul 23 2018, 23:45:11) 
[GCC 4.2.1 Compatible Apple LLVM 6.1.0 (clang-602.0.53)]
0.9.5
```


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/169?src=pr&el=h1) Report
> Merging [#169](https://codecov.io/gh/glotzerlab/signac/pull/169?src=pr&el=desc) into [master](https://codecov.io/gh/glotzerlab/signac/commit/88ede71b9d16a61c92f43f05c9662ef6725518b2?src=pr&el=desc) will **increase** coverage by `0.05%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/glotzerlab/signac/pull/169/graphs/tree.svg?width=650&token=FghbjTyzgM&height=150&src=pr)](https://codecov.io/gh/glotzerlab/signac/pull/169?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
+ Coverage    65.1%   65.16%   +0.05%     
==========================================
  Files          37       37              
  Lines        5505     5514       +9     
==========================================
+ Hits         3584     3593       +9     
  Misses       1921     1921
```


| [Impacted Files](https://codecov.io/gh/glotzerlab/signac/pull/169?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [signac/contrib/collection.py](https://codecov.io/gh/glotzerlab/signac/pull/169/diff?src=pr&el=tree#diff-c2lnbmFjL2NvbnRyaWIvY29sbGVjdGlvbi5weQ==) | `88.97% <100%> (+0.18%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/glotzerlab/signac/pull/169?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/glotzerlab/signac/pull/169?src=pr&el=footer). Last update [88ede71...bc65c2f](https://codecov.io/gh/glotzerlab/signac/pull/169?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

tcmoore3: > I have one change request to simplify the code logic, but otherwise looks got to me overall.
> 
> Did you do some research on whether our `round(value) == value` is best practice?

I did not. The other way to do it is to use the `float.is_integer()` function (introduced in 2.6, see https://stackoverflow.com/questions/21583758/how-to-check-if-a-float-value-is-a-whole-number?answertab=votes#tab-top). I'm not sure which is the best to use in this case.
csadorf: > > I have one change request to simplify the code logic, but otherwise looks got to me overall.
> > Did you do some research on whether our `round(value) == value` is best practice?
> 
> I did not. The other way to do it is to use the `float.is_integer()` function (introduced in 2.6, see [this discussion](https://stackoverflow.com/questions/21583758/how-to-check-if-a-float-value-is-a-whole-number?answertab=votes#tab-top)). So something like
> 
> ```python
> if isinstance(value, Number) and float(value).is_integer():
>     # do integer stuff
> ```
> 
> I'm not sure which is the best to use in this case.

I like `float.is_integer()`. We can simplify to: `if type(value) is float and value.is_integer():`

We can use the direct type check instead of `isinstance` as a micro optimization, because the filter argument is normalized and any floating-point type is guaranteed to be of type `float` at this point.
tcmoore3: > I like `float.is_integer()`. We can simplify to: `if type(value) is float and value.is_integer():`

I don't think that will work. We want to find both `int`s and `float`s when we search for either `int`s or integer-valued `float`s, so . E.g., both `signac find x 4` and `signac find x 4.0` should return jobs where `sp.x` is either 4 or 4.0.
csadorf: > > I like `float.is_integer()`. We can simplify to: `if type(value) is float and value.is_integer():`
> 
> I don't think that will work. We want to find both `int`s and `float`s when we search for either `int`s or integer-valued `float`s. E.g., both `signac find x 4` and `signac find x 4.0` should return jobs where `sp.x` is either 4 or 4.0.

Yeah, you're right. Feel free to leave as is then!
tcmoore3: > > > I like `float.is_integer()`. We can simplify to: `if type(value) is float and value.is_integer():`
> > 
> > 
> > I don't think that will work. We want to find both `int`s and `float`s when we search for either `int`s or integer-valued `float`s. E.g., both `signac find x 4` and `signac find x 4.0` should return jobs where `sp.x` is either 4 or 4.0.
> 
> Yeah, you're right. Feel free to leave as is then!

I also like `is_integer()` more than `round(x)==x`, so I changed it to use `is_integer`. You may want to double check the changes to the tests, but otherwise this PR should be good to go.

Patch:
diff --git a/changelog.txt b/changelog.txt
index 5845ceaf..753203ee 100644
--- a/changelog.txt
+++ b/changelog.txt
@@ -20,6 +20,7 @@ next
 ----
 
  - Add command line options ``--sp`` and ``--doc`` for ``signac find`` that allow users to display key-value pairs of the state point and document in combination with the job id (#97, #146).
+ - Fix: Searches for whole numbers will match all numerically matching integers regardless of whether they are stored as decimals or whole numbers (#169).
 
 
 [1.0.0] -- 2019-02-28
diff --git a/contributors.txt b/contributors.txt
index 2428d45d..3d92714b 100644
--- a/contributors.txt
+++ b/contributors.txt
@@ -7,3 +7,4 @@ Bradley Dice
 Tim Moore
 Pengji Zhou
 Eric Harper
+Kelly Wang
diff --git a/signac/contrib/collection.py b/signac/contrib/collection.py
index ab733539..23df2fde 100644
--- a/signac/contrib/collection.py
+++ b/signac/contrib/collection.py
@@ -12,13 +12,14 @@
 # on files on the local file system instead of a MongoDB database.
 #
 # [1]: https://github.com/mongodb/mongo-python-driver
-import sys
+import argparse
 import io
-import re
 import logging
-import argparse
 import operator
+import re
+import sys
 from itertools import islice
+from numbers import Number
 
 from ..core import json
 from ..common import six
@@ -30,7 +31,6 @@ else:
     from collections.abc import Mapping
 
 if six.PY2 or (six.PY3 and sys.version_info.minor < 5):
-
     def isclose(a, b, rel_tol=1e-9, abs_tol=0.0):
         return abs(a-b) <= max(rel_tol * max(abs(a), abs(b)), abs_tol)
 else:
@@ -620,7 +620,21 @@ class Collection(object):
                 raise KeyError("Unknown expression-operator '{}'.".format(op))
         else:
             index = self.index(key, build=True)
-            return index.get(value, set())
+            # Check to see if 'value' is a floating point type but an
+            # integer value (e.g., 4.0), and search for both the int and float
+            # values. This allows the user to find statepoints that have
+            # integer-valued keys that are stored as floating point types.
+            # Note that this both cases: 1) user searches for an int and hopes
+            # to find values that are stored as integer-valued floats and 2) user
+            # searches for a integer-valued float and hopes to find ints.
+            # This way, both `signac find x 4.0` and `signac find x 4` would
+            # return jobs where `sp.x` is stored as either 4.0 or 4.
+            if isinstance(value, Number) and float(value).is_integer():
+                result_float = index.get(_float(value), set())
+                result_int = index.get(int(value), set())
+                return result_int.union(result_float)
+            else:
+                return index.get(value, set())
 
     def _find_result(self, expr):
         if not len(expr):


Test patch:
diff --git a/tests/test_collection.py b/tests/test_collection.py
index 9c3fa7d4..8431545c 100644
--- a/tests/test_collection.py
+++ b/tests/test_collection.py
@@ -135,12 +135,14 @@ class CollectionTest(unittest.TestCase):
     def test_int_float_equality(self):
         self.c.insert_one(dict(a=1))
         self.c.insert_one(dict(a=1.0))
-        self.assertEqual(len(self.c.find(dict(a=1))), 1)
-        self.assertEqual(len(self.c.find(dict(a=1.0))), 1)
+        self.assertEqual(len(self.c.find(dict(a=1))), 2)
+        self.assertEqual(len(self.c.find(dict(a=1.0))), 2)
+        """
         for doc in self.c.find(dict(a=1)):
             self.assertEqual(type(doc['a']), int)
         for doc in self.c.find(dict(a=1.0)):
             self.assertEqual(type(doc['a']), float)
+        """
 
     def test_copy(self):
         docs = [dict(_id=str(i)) for i in range(10)]
@@ -259,7 +261,7 @@ class CollectionTest(unittest.TestCase):
         self.c.update(docs)
         self.assertEqual(len(self.c.find()), len(docs))
         self.assertEqual(len(self.c.find({'a': 0})), 1)
-        self.assertEqual(len(self.c.find({'a': 0.0})), 0)
+        self.assertEqual(len(self.c.find({'a': 0.0})), 1)
         self.assertEqual(list(self.c.find({'a': 0}))[0], docs[0])
         self.assertEqual(len(self.c.find({'a': -1})), 0)
         self.assertEqual(len(self.c.find({'a.b': 0})), 0)
@@ -276,7 +278,7 @@ class CollectionTest(unittest.TestCase):
         docs = [dict(a=float(i)) for i in range(10)]
         self.c.update(docs)
         self.assertEqual(len(self.c.find()), len(docs))
-        self.assertEqual(len(self.c.find({'a': 0})), 0)
+        self.assertEqual(len(self.c.find({'a': 0})), 1)
         self.assertEqual(len(self.c.find({'a': 0.0})), 1)
         self.assertEqual(list(self.c.find({'a': 0.0}))[0], docs[0])
         self.assertEqual(len(self.c.find({'a': -1})), 0)