Remove support for non-entity_id grouping for features #887
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Specifying
groups
other thanentity_id
has been error-prone because it requires (but doesn't enforce/check for) a one-to-one relationship between the entities and other grouping levels (discussed in #874). As such, this PR removes support for specifying thegroups
key in the feature configuration (as well as removing instances where this is done/discussed in the tests and docs), bumping the config version tov8
as result.Notably, this doesn't change the underlying
collate
code itself -- there doesn't seem to be anything special there about the entity-level grouping asgroups
is simply passed as a list which is iterated over in a few places. Restricting the list to just["entity_id"]
means a few loops are just traversed once but doesn't result in a bunch of cruft/untouched code, and it hardly seems worth simply removing the for loops.A couple of incidental/unrelated changes as well, I updated documentation to reflect the python 3.7+ requirement as well as the python version used by
./develop
to 3.9.10.Also, @ecsalomon -- once we're ready to merge both this and #883, we should be sure to update the
upgrade-to-v8.md
documentation to reflect both this change and specifying SQL files for the labels/cohort configs.