Skip to content

BUG: fix calling local references with keyword arguments in query #26426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 19, 2019

Conversation

danielhrisca
Copy link
Contributor

@danielhrisca danielhrisca commented May 16, 2019

  • closes #xxxx
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

calling local references with keyword arguments in query fails with attribute error:

import numpy as np
import pandas as pd

def f(a, b):
    return b

df = pd.DataFrame()

df['val'] = np.arange(10)
print(df.query('val > @f(4, 5)'))
print(df.query('val > @f(4, b=5)'))
   val
6    6
7    7
8    8
9    9
Traceback (most recent call last):

  File "C:/Users/uidn3651/Desktop/untitled0.py", line 10, in <module>
    print(df.query('val > @f(4, b=5)'))

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\frame.py", line 3052, in query
    res = self.eval(expr, **kwargs)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\frame.py", line 3169, in eval
    return _eval(expr, inplace=inplace, **kwargs)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\eval.py", line 293, in eval
    truediv=truediv)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 749, in __init__
    self.terms = self.parse()

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 766, in parse
    return self._visitor.visit(self.expr)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 370, in visit
    return visitor(node, **kwargs)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 376, in visit_Module
    return self.visit(expr, **kwargs)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 370, in visit
    return visitor(node, **kwargs)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 379, in visit_Expr
    return self.visit(node.value, **kwargs)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 370, in visit
    return visitor(node, **kwargs)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 674, in visit_Compare
    return self.visit(binop)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 370, in visit
    return visitor(node, **kwargs)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 476, in visit_BinOp
    op, op_class, left, right = self._maybe_transform_eq_ne(node)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 413, in _maybe_transform_eq_ne
    right = self.visit(node.right, side='right')

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 370, in visit
    return visitor(node, **kwargs)

  File "d:\work\02__pythonworkspace\_venv\lib\site-packages\pandas\core\computation\expr.py", line 657, in visit_Call
    kwargs.append(ast.keyword(

AttributeError: 'dict' object has no attribute 'append'

With the fix this works correctly

   val
6    6
7    7
8    8
9    9
   val
6    6
7    7
8    8
9    9

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests always are first

@danielhrisca danielhrisca changed the title fix calling local references with keyword arguments in query BUG: fix calling local references with keyword arguments in query May 16, 2019
@codecov
Copy link

codecov bot commented May 16, 2019

Codecov Report

Merging #26426 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26426      +/-   ##
==========================================
- Coverage   91.74%   91.73%   -0.01%     
==========================================
  Files         174      174              
  Lines       50748    50748              
==========================================
- Hits        46557    46556       -1     
- Misses       4191     4192       +1
Flag Coverage Δ
#multiple 90.25% <100%> (+0.01%) ⬆️
#single 41.69% <0%> (-0.09%) ⬇️
Impacted Files Coverage Δ
pandas/core/computation/expr.py 97.52% <100%> (+0.82%) ⬆️
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 97.02% <0%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f44ac06...c7d58af. Read the comment docs.

@danielhrisca
Copy link
Contributor Author

@jreback

running test_fast.bat produces many failed tests. However I can't find test_multi_line_expression_callable_local_variable in the output. Does this mean that this test is passed?

@danielhrisca
Copy link
Contributor Author

@jreback
can you have a look at the new commit? Thank you

@gfyoung gfyoung added the Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff label May 18, 2019
""", inplace=True)
assert_frame_equal(expected, df)
assert ans is None

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split this test into two.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gfyoung
done

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a whatsnew note; reshaping or other section would be ok

@jreback
Copy link
Contributor

jreback commented May 19, 2019

you will need to merge master as well, there is a conflict

@jreback jreback added this to the 0.25.0 milestone May 19, 2019
@danielhrisca
Copy link
Contributor Author

Th conflict is now resolved

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a different format for the issue number

@@ -380,6 +379,7 @@ Indexing
- Bug in which :meth:`DataFrame.append` produced an erroneous warning indicating that a ``KeyError`` will be thrown in the future when the data to be appended contains new columns (:issue:`22252`).
- Bug in which :meth:`DataFrame.to_csv` caused a segfault for a reindexed data frame, when the indices were single-level :class:`MultiIndex` (:issue:`26303`).
- Fixed bug where assigning a :class:`arrays.PandasArray` to a :class:`pandas.core.frame.DataFrame` would raise error (:issue:`26390`)
- Allow keyword arguments for callable local reference used in the :method:`DataFrame.query` string (PR 26426)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be like the other, e.g. (:issue:`26426`)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thank you for the patience!

@jreback jreback merged commit e4f727b into pandas-dev:master May 19, 2019
@jreback
Copy link
Contributor

jreback commented May 19, 2019

thanks for the patch @danielhrisca

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants