Add commutative argument to SubtractNumeric and DivideNumeric primitives #457
Conversation
Codecov Report
@@ Coverage Diff @@
## master #457 +/- ##
==========================================
+ Coverage 96.46% 96.46% +<.01%
==========================================
Files 98 98
Lines 8620 8623 +3
==========================================
+ Hits 8315 8318 +3
Misses 305 305
Continue to review full report at Codecov.
|
| @@ -230,7 +230,9 @@ class SubtractNumeric(TransformPrimitive): | |||
| name = "subtract_numeric" | |||
There was a problem hiding this comment.
can we add a docstring to this class? Something like this
"""Subtracts one feature from another and returns the difference
Args:
commutative (bool): determines if Deep Feature Synthesis should generate
both a - b and b - a, or just one. If commutative is False, there is no
guarantee which of the two will be generated. Defaults to True.
"""
There was a problem hiding this comment.
@kmax12 it looks strange to add docstring to SubtractNumeric and DivideNumeric classes only. All other primitives don't have docstrings. What do you think?
There was a problem hiding this comment.
it's only in binary_transform.py that don't have docstrings. the other primitive files do.
we didn't put them here since most people aren't importing and using the binary primitives directly. however, now that you added a parameter, someone will need to know how to use it.
if you have time, you're more than welcome to add docstrings to the other methods in binary_transform.py!
| @@ -308,6 +310,9 @@ class DivideNumeric(TransformPrimitive): | |||
| input_types = [Numeric, Numeric] | |||
There was a problem hiding this comment.
Please add a docstring for this primitive class too. It can be siimilar to the subtract example I gave
|
@grayskripko can you sign the CLA so I can merge this in? I think you can do it by clicking here: https://cla-assistant.io/Featuretools/featuretools?pullRequest=457 |
I've done it now but, actually, it was the second time |
|
You're right. I see you also did sign it on March 6th. Not sure why it didn't update it here. Will go ahead and merge now |
kmax12
changed the title
Add commutative argument to SubtractNumeric and DivideNumeric primitives