Repository: glotzerlab/signac

Issue:
groupby() by variables that not present in the entire data space
### Feature description

I am interested in obtaining all the jobs that contain a variable `b` which does not necessarily have to be present in all the jobs.


### Proposed solution
From @bdice in the slack group:

> Bradley Dice  10:43 AM
@Miguel This behavior could be improved, but what would you expect the behavior to be? A group where b is None isn't really an option because b could be set to None in the statepoint. ***We could return only groups where the value is set (and ignore jobs where the key is missing).***

### Additional context
Example of the current problem. First `groupby()` works, second does not.
```python
import signac

project = signac.init_project('test')
sp = { 'a' : 1.0,
       'c' : 2.0
        }
job = project.open_job(sp)
job.init()

sp = { 'a' : 2.0,
       'b' : 1.0,
        }
job = project.open_job(sp)
job.init()

sp = { 'a' : 1.0,
       'b' : 3.0,
        }
job = project.open_job(sp)
job.init()

for a, group in project.groupby('a'):
    print(a, list(group))

for b, group in project.groupby('b'):
    print(b, list(group))
```


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

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

```diff
@@            Coverage Diff             @@
##           master     #323      +/-   ##
==========================================
+ Coverage   76.18%   76.21%   +0.02%     
==========================================
  Files          43       43              
  Lines        7076     7083       +7     
==========================================
+ Hits         5391     5398       +7     
  Misses       1685     1685              
```


| [Impacted Files](https://codecov.io/gh/glotzerlab/signac/pull/323?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [signac/contrib/project.py](https://codecov.io/gh/glotzerlab/signac/pull/323/diff?src=pr&el=tree#diff-c2lnbmFjL2NvbnRyaWIvcHJvamVjdC5weQ==) | `91.66% <100.00%> (+0.08%)` | :arrow_up: |

------

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

zhou-pj: LGTM！
zhou-pj: No necessary directly related to this PR, but I wonder if the following part in docstring (from line 1753) are still supported and tested in the code? I'm not sure I understand how `_doc_filter` is used as groupby key in this function.
```
#Group by job.sp['d'] and job.document['count'] using a lambda.
      for key, group in project.groupby(
                lambda job: (job.sp['d'], job.document['count'])
            ):
                print(key, list(group))```
vyasr: @zhou-pj that's a good question. The API here is actually a little bit strange. Right now, if you pass a string or group of strings to `groupby` or `groupbydoc`, they will automatically construct functions for grouping jobs by statepoint or document keys, respectively. However, you can also pass a callable. If you pass a callable to `groupby`, then it allows grouping by any job parameters because the calllable is a function of the job. If you pass a callable to `groupbydoc`, on the other hand, the callable must be a function of the _job document_. @bdice I believe you were responsible for the initial implementation of this feature, do you recall why we decided to split the grouping functionality into two functions that have this slightly divergent behavior?
bdice: @vyasr @zhou-pj I looked at the [original pull request](https://bitbucket.org/glotzer/signac/pull-requests/26/groupby/diff) where I introduced this feature. I don't recall discussing this API choice explicitly. I think it was an arbitrary choice that allowed `groupby` to be very general (a callable of `job` is more flexible than a callable of `job.sp`) while allowing `groupbydoc` to have `job.doc` as the argument for convenience. The most "parallel" solution would be to have `groupby` operate on `job.sp` in all cases but it would not be useful for cases where you want to group by something like `job.isfile('output.txt')`.
vyasr: In the interest of keeping this PR focused, I'm going to move this discussion to a separate issue.
vyasr: @lyrivera yes, codecov changes are pretty noisy and frequently fail showing negligible changes in coverage i.e. -0.05%. That number is reported as part of the check, so you can do a sanity check that it isn't actually dropping by >1% or something (which would be a more plausible indication that lots of untested code was added).

@lyrivera @zhou-pj if you're happy, can you actually approve the PR?

@csadorf you had some comments when reviewing this PR, so I'd prefer not to have this merged without your approval. Please let us know if you're happy with it now.

Patch:
diff --git a/changelog.txt b/changelog.txt
index 91e497f3..69cf687c 100644
--- a/changelog.txt
+++ b/changelog.txt
@@ -15,9 +15,11 @@ Added
 
  - Type annotations are validated during continuous integration (#313).
  - Added ``_repr_html_`` method in ``ProjectSchema`` class (#314, #324).
+ - Allow grouping by variables that are not present in all jobs in the project in ``JobsCursor.groupby`` (#321, #323).
 
 Fixed
 +++++
+
  - Fix the ``signac config verify`` command (previously broken) (#301, #302).
  - Warnings now appear when raised by the ``signac`` CLI (#317, #308).
 
diff --git a/signac/contrib/project.py b/signac/contrib/project.py
index c536c40a..9d85724d 100644
--- a/signac/contrib/project.py
+++ b/signac/contrib/project.py
@@ -1776,8 +1776,14 @@ class JobsCursor(object):
             A default value to be used when a given state point key is not present (must
             be sortable).
         """
+        _filter = self._filter
         if isinstance(key, str):
             if default is None:
+                if _filter is None:
+                    _filter = {key: {"$exists": True}}
+                else:
+                    _filter = {'$and': [{key: {"$exists": True}}, _filter]}
+
                 def keyfunction(job):
                     return job.sp[key]
             else:
@@ -1786,6 +1792,11 @@ class JobsCursor(object):
 
         elif isinstance(key, Iterable):
             if default is None:
+                if _filter is None:
+                    _filter = {k: {"$exists": True} for k in key}
+                else:
+                    _filter = {'$and': [{k: {"$exists": True} for k in key}, _filter]}
+
                 def keyfunction(job):
                     return tuple(job.sp[k] for k in key)
             else:
@@ -1800,7 +1811,8 @@ class JobsCursor(object):
         else:
             keyfunction = key
 
-        return groupby(sorted(iter(self), key=keyfunction), key=keyfunction)
+        return groupby(sorted(iter(JobsCursor(self._project, _filter, self._doc_filter)),
+                              key=keyfunction), key=keyfunction)
 
     def groupbydoc(self, key=None, default=None):
         """Groups jobs according to one or more document values.


Test patch:
diff --git a/tests/test_project.py b/tests/test_project.py
index a128dfd9..100617bc 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -836,9 +836,7 @@ class TestProject(TestProjectBase):
             assert len(list(g)) == 6
             for job in list(g):
                 assert job.sp['b'] == k
-        with pytest.raises(KeyError):
-            for k, g in self.project.groupby('d'):
-                pass
+        assert len(list(self.project.groupby('d'))) == 0
         for k, g in self.project.groupby('d', default=-1):
             assert k == -1
             assert len(list(g)) == len(self.project)
@@ -863,6 +861,17 @@ class TestProject(TestProjectBase):
                 assert str(job) == k
         assert group_count == len(list(self.project.find_jobs()))
 
+        self.project.open_job({'a': 20}).init()
+        for k, g in self.project.groupby('b'):
+            assert len(list(g)) == 6
+            for job in list(g):
+                assert job.sp['b'] == k
+        for k, g in self.project.groupby(('b', 'c')):
+            assert len(list(g)) == 2
+            for job in list(g):
+                assert job.sp['b'] == k[0]
+                assert job.sp['c'] == k[1]
+
     def test_jobs_groupbydoc(self):
         def get_doc(i):
             return {