Repository: deppen8/pandas-vet

Issue:
Arithmetic and comparison operators
We should implement checks for all of the text-based arithmetic and comparison operators. If found, recommend using the operator itself. Something like:

> Use <operator> instead of <text method>

| use | check for |
| --- | ---------- |
| `+` | `.add` |
| `-` | `.sub` and `.subtract` |
| `*` | `.mul` and `.multiply` |
| `/` | `.div`, `.divide` and `.truediv` |
| `**` | `.pow` |
| `//` | `.floordiv` |
| `%` | `.mod` |
| `>` | `.gt` |
| `<` | `.lt` |
| `>=` | `.ge` |
| `<=` | `.le` |
| `==` | `.eq` |
| `!=` | `.ne` |

Discussion:
simchuck: I'm not understanding how the CI process works, and what the build failure means.  How can I fix this?
deppen8: > I'm not understanding how the CI process works, and what the build failure means. How can I fix this?

If you click on the "Details" link next to the Travis message here on GitHub, it should open the Travis dashboard for this project. Below the main info, you should see two Build Jobs listed: one for Python 3.5 and one for 3.6. Click on either one of those bars and it will expand and show you the actual command line from that build attempt.

In your case, the errors are actually different for the different Python versions. 
- Python 3.5 raised an error because f'strings sadly didn't get implemented until 3.6. 
- Python 3.6 raised an error because `node.func.attr` is not behaving as expected. `node.func` is returning some kind of `Name` object.

Patch:
diff --git a/pandas_vet/__init__.py b/pandas_vet/__init__.py
index 8c286e0..b675fd8 100644
--- a/pandas_vet/__init__.py
+++ b/pandas_vet/__init__.py
@@ -14,6 +14,9 @@ class Visitor(ast.NodeVisitor):
     ast.NodeVisitor will automatically call the appropriate method for a given node type
 
     i.e. calling self.visit on an Import node calls visit_import
+
+    The `check` functions should be called from the `visit_` method that
+    would produce a 'fail' condition.  
     """
     errors = attr.ib(default=attr.Factory(list))
 
@@ -28,6 +31,8 @@ class Visitor(ast.NodeVisitor):
         self.errors.extend(check_for_notnull(node))
         self.errors.extend(check_for_pivot(node))
         self.errors.extend(check_for_unstack(node))
+        self.errors.extend(check_for_arithmetic_methods(node))
+        self.errors.extend(check_for_comparison_methods(node))
 
     def visit_Subscript(self, node):
         self.generic_visit(node)  # continue checking children
@@ -86,6 +91,50 @@ def check_for_notnull(node: ast.Call) -> List:
         return [PD004(node.lineno, node.col_offset)]
     return []
 
+def check_for_arithmetic_methods(node: ast.Call) -> List:
+    """
+    Check AST for occurence of explicit arithmetic methods.  
+
+    Error/warning message to recommend use of binary arithmetic operators instead.
+    """
+    arithmetic_methods = [
+        'add',
+        'sub', 'subtract',
+        'mul', 'multiply',
+        'div', 'divide', 'truediv',
+        'pow',
+        'floordiv',
+        'mod',
+        ]
+    arithmetic_operators = [
+        '+',
+        '-',
+        '*',
+        '/',
+        '**',
+        '//',
+        '%',
+        ]
+
+    if isinstance(node.func, ast.Attribute) and node.func.attr in arithmetic_methods:
+        return [PD005(node.lineno, node.col_offset)]
+    return []
+
+
+def check_for_comparison_methods(node: ast.Call) -> List:
+    """
+    Check AST for occurence of explicit comparison methods.  
+
+    Error/warning message to recommend use of binary comparison operators instead.
+    """
+    comparison_methods = ['gt', 'lt', 'ge', 'le', 'eq', 'ne']
+    comparison_operators = ['>',  '<',  '>=', '<=', '==', '!=']
+
+    if isinstance(node.func, ast.Attribute) and node.func.attr in comparison_methods:
+        return [PD006(node.lineno, node.col_offset)]
+    return []
+
+
 def check_for_ix(node: ast.Subscript) -> List:
     if node.value.attr == "ix":
         return [PD007(node.lineno, node.col_offset)]


Test patch:
diff --git a/tests/test_PD005.py b/tests/test_PD005.py
new file mode 100644
index 0000000..9c68e0e
--- /dev/null
+++ b/tests/test_PD005.py
@@ -0,0 +1,51 @@
+"""
+Test to check for use of explicit arithmetic methods.
+
+Recommend use of binary arithmetic operators instead.
+"""
+import ast
+
+from pandas_vet import VetPlugin
+from pandas_vet import PD005
+
+
+def test_PD005_pass_arithmetic_operator():
+    """
+    Test that using binary arithmetic operator explicitly does not result in an error.
+    """
+    arithmetic_operators = [
+        '+',
+        '-',
+        '*',
+        '/',
+        '**',
+        '//',
+        '%',
+        ]
+    for op in arithmetic_operators:
+        statement = 'C = A {0} B'.format(op)
+        tree = ast.parse(statement)
+        actual = list(VetPlugin(tree).run())
+        expected = []
+        assert actual == expected
+
+
+def test_PD005_fail_arithmetic_method():
+    """
+    Test that using arithmetic method results in an error.
+    """
+    arithmetic_methods = [
+        'add',
+        'sub', 'subtract',
+        'mul', 'multiply',
+        'div', 'divide', 'truediv',
+        'pow',
+        'floordiv',
+        'mod',
+        ]
+    for op in arithmetic_methods:
+        statement = 'C = A.{0}(B)'.format(op)
+        tree = ast.parse(statement)
+        actual = list(VetPlugin(tree).run())
+        expected = [PD005(1, 4)]
+        assert actual == expected
diff --git a/tests/test_PD006.py b/tests/test_PD006.py
new file mode 100644
index 0000000..cb30365
--- /dev/null
+++ b/tests/test_PD006.py
@@ -0,0 +1,35 @@
+"""
+Test to check for use of explicit comparison methods.
+
+Recommend use of binary comparison operators instead.
+"""
+import ast
+
+from pandas_vet import VetPlugin
+from pandas_vet import PD006
+
+
+def test_PD006_pass_comparison_operator():
+    """
+    Test that using binary comparison operator explicitly does not result in an error.
+    """
+    comparison_operators = ['>',  '<',  '>=', '<=', '==', '!=']
+    for op in comparison_operators:
+        statement = 'C = A {0} B'.format(op)
+        tree = ast.parse(statement)
+        actual = list(VetPlugin(tree).run())
+        expected = []
+        assert actual == expected
+
+
+def test_PD006_fail_comparison_method():
+    """
+    Test that using comparison method results in an error.
+    """
+    comparison_methods = ['gt', 'lt', 'ge', 'le', 'eq', 'ne']
+    for op in comparison_methods:
+        statement = 'C = A.{0}(B)'.format(op)
+        tree = ast.parse(statement)
+        actual = list(VetPlugin(tree).run())
+        expected = [PD006(1, 4)]
+        assert actual == expected