Primitives as strings in DFS parameters #129
Conversation
bschreck
requested review from
Seth-Rothschild and
kmax12
and removed request for
Seth-Rothschild
|
Very excited for this. I can take a last pass through the docs once you're all set. |
|
@Seth-Rothschild ready for you to do that |
| transform_primitives = get_transform_primitives() | ||
| agg_primitives = get_aggregation_primitives() | ||
| transform_df = pd.DataFrame({'name': list(transform_primitives.keys()), | ||
| 'description': [prim.__doc__.split("\n")[0] for prim in transform_primitives.values()]}) |
There was a problem hiding this comment.
This gives different results between docstrings that are
"""
Words go here
"""
"""Words go here
"""
and
""" Words go here
"""
We have all three but should probably settle on one.
There was a problem hiding this comment.
@Seth-Rothschild we use the Google style docstrings for all public functions. for some private functions, we may be inconsistent and should fix those as we see it.
here's the Google style docstrings guide: http://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html
There was a problem hiding this comment.
I've pushed a version which has nonempty first line for all of the primitives. It looks like there's some extra space being added by something in the description (as shown in this image).
EDIT: It's actually right aligned. Pandas style doesn't work with a non-unique index, but if we give it a unique index we can use .style.set_properties(**{'text-align': 'left'})
Codecov Report
@@ Coverage Diff @@
## master #129 +/- ##
==========================================
- Coverage 88.47% 88.36% -0.12%
==========================================
Files 73 73
Lines 7558 7598 +40
==========================================
+ Hits 6687 6714 +27
- Misses 871 884 +13
Continue to review full report at Codecov.
|
| where_primitives (list[:class:`.primitives.AggregationPrimitive`], optional): | ||
| only add where clauses to these types of Aggregation | ||
| Features. | ||
| Default: ["sum", \ |
There was a problem hiding this comment.
Nitpick - I dont think these all need their own line. can we reform to be 80 chars?
There was a problem hiding this comment.
It's 114 chars (agg defaults) and 104 chars (trans defaults). Keep it on one line anyways? Even removing " Default:" isn't enough to get it under 80.
| if isinstance(p, basestring): | ||
| prim_obj = agg_prim_dict.get(p.lower(), None) | ||
| if prim_obj is None: | ||
| prim_obj = trans_prim_dict.get(p.lower(), None) |
There was a problem hiding this comment.
where's only apply to aggregations, no need to check transform too
featuretools/synthesis/dfs.py
Outdated
| :class:`Mode <.primitives.Mode>`] | ||
|
|
||
| trans_primitives (list[TransformPrimitive], optional): | ||
| Default: ["sum", \ |
There was a problem hiding this comment.
same comment as above about single lines
|
Great work. Merging in |
**v0.1.20** Apr 13, 2018 * Improved chunking when calculating feature matrices (#121) * Primitives as strings in DFS parameters (#129) * Integer time index bugfixes (#128) * Add make_temporal_cutoffs utility function (#126) * Show all entities, switch shape display to row/col (#124) * fixed num characters nan fix (#118) * modify ignore_variables docstring (#117)
dfs() and DeepFeatureSynthesis() can now accept strings (instead of Python classes) for all primitives built into Featuretools.
API looks like this: