From 7c726e9997bcb3f2802a43047ab5564707deaa74 Mon Sep 17 00:00:00 2001 From: aram-cinnamon <97805700+aram-cinnamon@users.noreply.github.com> Date: Tue, 20 Aug 2024 16:08:18 -0400 Subject: [PATCH 1/2] BUG: `query` on columns with characters like # in its name (#59296) --- .pre-commit-config.yaml | 4 +- doc/source/whatsnew/v3.0.0.rst | 1 + pandas/core/computation/parsing.py | 133 +++++++++++++++++--- pandas/core/frame.py | 14 +-- pandas/tests/frame/test_query_eval.py | 167 ++++++++++++++++++++++++-- 5 files changed, 277 insertions(+), 42 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b81b9ba070a44e..f6717dd503c9be 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -23,6 +23,7 @@ repos: hooks: - id: ruff args: [--exit-non-zero-on-fix] + exclude: ^pandas/tests/frame/test_query_eval.py - id: ruff # TODO: remove autofixe-only rules when they are checked by ruff name: ruff-selected-autofixes @@ -31,7 +32,7 @@ repos: exclude: ^pandas/tests args: [--select, "ANN001,ANN2", --fix-only, --exit-non-zero-on-fix] - id: ruff-format - exclude: ^scripts + exclude: ^scripts|^pandas/tests/frame/test_query_eval.py - repo: https://github.com/jendrikseipp/vulture rev: 'v2.11' hooks: @@ -85,6 +86,7 @@ repos: types: [text] # overwrite types: [rst] types_or: [python, rst] - id: rst-inline-touching-normal + exclude: ^pandas/tests/frame/test_query_eval.py types: [text] # overwrite types: [rst] types_or: [python, rst] - repo: https://github.com/sphinx-contrib/sphinx-lint diff --git a/doc/source/whatsnew/v3.0.0.rst b/doc/source/whatsnew/v3.0.0.rst index 57b85b933f0207..bdeb9c48990a2d 100644 --- a/doc/source/whatsnew/v3.0.0.rst +++ b/doc/source/whatsnew/v3.0.0.rst @@ -685,6 +685,7 @@ Other - Bug in :meth:`DataFrame.apply` where passing ``engine="numba"`` ignored ``args`` passed to the applied function (:issue:`58712`) - Bug in :meth:`DataFrame.eval` and :meth:`DataFrame.query` which caused an exception when using NumPy attributes via ``@`` notation, e.g., ``df.eval("@np.floor(a)")``. (:issue:`58041`) - Bug in :meth:`DataFrame.eval` and :meth:`DataFrame.query` which did not allow to use ``tan`` function. (:issue:`55091`) +- Bug in :meth:`DataFrame.query` which raised an exception or produced incorrect results when expressions contained backtick-quoted column names containing the hash character ``#``, backticks, or characters that fall outside the ASCII range (U+0001..U+007F). (:issue:`59285`) (:issue:`49633`) - Bug in :meth:`DataFrame.sort_index` when passing ``axis="columns"`` and ``ignore_index=True`` and ``ascending=False`` not returning a :class:`RangeIndex` columns (:issue:`57293`) - Bug in :meth:`DataFrame.transform` that was returning the wrong order unless the index was monotonically increasing. (:issue:`57069`) - Bug in :meth:`DataFrame.where` where using a non-bool type array in the function would return a ``ValueError`` instead of a ``TypeError`` (:issue:`56330`) diff --git a/pandas/core/computation/parsing.py b/pandas/core/computation/parsing.py index 8fbf8936d31efc..35a6d1c6ad269f 100644 --- a/pandas/core/computation/parsing.py +++ b/pandas/core/computation/parsing.py @@ -4,6 +4,7 @@ from __future__ import annotations +from enum import Enum from io import StringIO from keyword import iskeyword import token @@ -32,13 +33,21 @@ def create_valid_python_identifier(name: str) -> str: ------ SyntaxError If the returned name is not a Python valid identifier, raise an exception. - This can happen if there is a hashtag in the name, as the tokenizer will - than terminate and not find the backtick. - But also for characters that fall out of the range of (U+0001..U+007F). """ if name.isidentifier() and not iskeyword(name): return name + # Escape characters that fall outside the ASCII range (U+0001..U+007F). + # GH 49633 + gen = ( + (c, "".join(chr(b) for b in c.encode("ascii", "backslashreplace"))) + for c in name + ) + name = "".join( + c_escaped.replace("\\", "_UNICODE_" if c != c_escaped else "_BACKSLASH_") + for c, c_escaped in gen + ) + # Create a dict with the special characters and their replacement string. # EXACT_TOKEN_TYPES contains these special characters # token.tok_name contains a readable description of the replacement string. @@ -54,11 +63,10 @@ def create_valid_python_identifier(name: str) -> str: "$": "_DOLLARSIGN_", "€": "_EUROSIGN_", "°": "_DEGREESIGN_", - # Including quotes works, but there are exceptions. "'": "_SINGLEQUOTE_", '"': "_DOUBLEQUOTE_", - # Currently not possible. Terminates parser and won't find backtick. - # "#": "_HASH_", + "#": "_HASH_", + "`": "_BACKTICK_", } ) @@ -127,6 +135,9 @@ def clean_column_name(name: Hashable) -> Hashable: which is not caught and propagates to the user level. """ try: + # Escape backticks + name = name.replace("`", "``") if isinstance(name, str) else name + tokenized = tokenize_string(f"`{name}`") tokval = next(tokenized)[1] return create_valid_python_identifier(tokval) @@ -168,6 +179,91 @@ def tokenize_backtick_quoted_string( return BACKTICK_QUOTED_STRING, source[string_start:string_end] +class ParseState(Enum): + DEFAULT = 0 + IN_BACKTICK = 1 + IN_SINGLE_QUOTE = 2 + IN_DOUBLE_QUOTE = 3 + + +def _split_by_backtick(s: str) -> list[tuple[bool, str]]: + """ + Splits a str into substrings along backtick characters (`). + + Disregards backticks inside quotes. + + Parameters + ---------- + s : str + The Python source code string. + + Returns + ------- + substrings: list[tuple[bool, str]] + List of tuples, where each tuple has two elements: + The first is a boolean indicating if the substring is backtick-quoted. + The second is the actual substring. + """ + substrings = [] + substr: list[str] = [] # Will join into a string before adding to `substrings` + i = 0 + parse_state = ParseState.DEFAULT + while i < len(s): + char = s[i] + + match char: + case "`": + # start of a backtick-quoted string + if parse_state == ParseState.DEFAULT: + if substr: + substrings.append((False, "".join(substr))) + + substr = [char] + i += 1 + parse_state = ParseState.IN_BACKTICK + continue + + elif parse_state == ParseState.IN_BACKTICK: + # escaped backtick inside a backtick-quoted string + next_char = s[i + 1] if (i != len(s) - 1) else None + if next_char == "`": + substr.append(char) + substr.append(next_char) + i += 2 + continue + + # end of the backtick-quoted string + else: + substr.append(char) + substrings.append((True, "".join(substr))) + + substr = [] + i += 1 + parse_state = ParseState.DEFAULT + continue + case "'": + # start of a single-quoted string + if parse_state == ParseState.DEFAULT: + parse_state = ParseState.IN_SINGLE_QUOTE + # end of a single-quoted string + elif (parse_state == ParseState.IN_SINGLE_QUOTE) and (s[i - 1] != "\\"): + parse_state = ParseState.DEFAULT + case '"': + # start of a double-quoted string + if parse_state == ParseState.DEFAULT: + parse_state = ParseState.IN_DOUBLE_QUOTE + # end of a double-quoted string + elif (parse_state == ParseState.IN_DOUBLE_QUOTE) and (s[i - 1] != "\\"): + parse_state = ParseState.DEFAULT + substr.append(char) + i += 1 + + if substr: + substrings.append((False, "".join(substr))) + + return substrings + + def tokenize_string(source: str) -> Iterator[tuple[int, str]]: """ Tokenize a Python source code string. @@ -182,18 +278,19 @@ def tokenize_string(source: str) -> Iterator[tuple[int, str]]: tok_generator : Iterator[Tuple[int, str]] An iterator yielding all tokens with only toknum and tokval (Tuple[ing, str]). """ + # GH 59285 + # Escape characters, including backticks + source = "".join( + ( + create_valid_python_identifier(substring[1:-1]) + if is_backtick_quoted + else substring + ) + for is_backtick_quoted, substring in _split_by_backtick(source) + ) + line_reader = StringIO(source).readline token_generator = tokenize.generate_tokens(line_reader) - # Loop over all tokens till a backtick (`) is found. - # Then, take all tokens till the next backtick to form a backtick quoted string - for toknum, tokval, start, _, _ in token_generator: - if tokval == "`": - try: - yield tokenize_backtick_quoted_string( - token_generator, source, string_start=start[1] + 1 - ) - except Exception as err: - raise SyntaxError(f"Failed to parse backticks in '{source}'.") from err - else: - yield toknum, tokval + for toknum, tokval, _, _, _ in token_generator: + yield toknum, tokval diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 1e6608b0d87f3b..b84fb33af26e5d 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -4556,17 +4556,8 @@ def query(self, expr: str, *, inplace: bool = False, **kwargs) -> DataFrame | No quoted string are replaced by strings that are allowed as a Python identifier. These characters include all operators in Python, the space character, the question mark, the exclamation mark, the dollar sign, and the euro sign. - For other characters that fall outside the ASCII range (U+0001..U+007F) - and those that are not further specified in PEP 3131, - the query parser will raise an error. - This excludes whitespace different than the space character, - but also the hashtag (as it is used for comments) and the backtick - itself (backtick can also not be escaped). - - In a special case, quotes that make a pair around a backtick can - confuse the parser. - For example, ```it's` > `that's``` will raise an error, - as it forms a quoted string (``'s > `that'``) with a backtick inside. + + A backtick can be escaped by double backticks. See also the `Python documentation about lexical analysis `__ @@ -4620,6 +4611,7 @@ def query(self, expr: str, *, inplace: bool = False, **kwargs) -> DataFrame | No raise ValueError(msg) kwargs["level"] = kwargs.pop("level", 0) + 1 kwargs["target"] = None + res = self.eval(expr, **kwargs) try: diff --git a/pandas/tests/frame/test_query_eval.py b/pandas/tests/frame/test_query_eval.py index aa2fb19fe85284..fa71153d011571 100644 --- a/pandas/tests/frame/test_query_eval.py +++ b/pandas/tests/frame/test_query_eval.py @@ -1,4 +1,5 @@ import operator +from tokenize import TokenError import numpy as np import pytest @@ -1246,6 +1247,8 @@ def df(self): "it's": [6, 3, 1], "that's": [9, 1, 8], "☺": [8, 7, 6], + "xy （z）": [1, 2, 3], # noqa: RUF001 + "xy （z\\uff09": [4, 5, 6], # noqa: RUF001 "foo#bar": [2, 4, 5], 1: [5, 7, 9], } @@ -1341,20 +1344,160 @@ def test_missing_attribute(self, df): with pytest.raises(AttributeError, match=message): df.eval("@pd.thing") - def test_failing_quote(self, df): - msg = r"(Could not convert ).*( to a valid Python identifier.)" - with pytest.raises(SyntaxError, match=msg): - df.query("`it's` > `that's`") + def test_quote(self, df): + res = df.query("`it's` > `that's`") + expect = df[df["it's"] > df["that's"]] + tm.assert_frame_equal(res, expect) - def test_failing_character_outside_range(self, df): - msg = r"(Could not convert ).*( to a valid Python identifier.)" - with pytest.raises(SyntaxError, match=msg): - df.query("`☺` > 4") + def test_character_outside_range_smiley(self, df): + res = df.query("`☺` > 4") + expect = df[df["☺"] > 4] + tm.assert_frame_equal(res, expect) - def test_failing_hashtag(self, df): - msg = "Failed to parse backticks" - with pytest.raises(SyntaxError, match=msg): - df.query("`foo#bar` > 4") + def test_character_outside_range_2_byte_parens(self, df): + # GH 49633 + res = df.query("`xy （z）` == 2") # noqa: RUF001 + expect = df[df["xy （z）"] == 2] # noqa: RUF001 + tm.assert_frame_equal(res, expect) + + def test_character_outside_range_and_actual_backslash(self, df): + # GH 49633 + res = df.query("`xy （z\\uff09` == 2") # noqa: RUF001 + expect = df[df["xy \uff08z\\uff09"] == 2] + tm.assert_frame_equal(res, expect) + + def test_hashtag(self, df): + res = df.query("`foo#bar` > 4") + expect = df[df["foo#bar"] > 4] + tm.assert_frame_equal(res, expect) + + def test_expr_with_column_name_with_hashtag_character(self): + # GH 59285 + df = DataFrame((1, 2, 3), columns=["a#"]) + result = df.query("`a#` < 2") + expected = df[df["a#"] < 2] + tm.assert_frame_equal(result, expected) + + def test_expr_with_comment(self): + # GH 59285 + df = DataFrame((1, 2, 3), columns=["a#"]) + result = df.query("`a#` < 2 # This is a comment") + expected = df[df["a#"] < 2] + tm.assert_frame_equal(result, expected) + + def test_expr_with_column_name_with_backtick_and_hash(self): + # GH 59285 + df = DataFrame((1, 2, 3), columns=["a`#b"]) + result = df.query("`a``#b` < 2") + expected = df[df["a`#b"] < 2] + tm.assert_frame_equal(result, expected) + + def test_expr_with_column_name_with_backtick(self): + # GH 59285 + df = DataFrame({"a`b": (1, 2, 3), "ab": (4, 5, 6)}) + result = df.query("`a``b` < 2") # noqa + # Note: Formatting checks may wrongly consider the above ``inline code``. + expected = df[df["a`b"] < 2] + tm.assert_frame_equal(result, expected) + + @pytest.mark.xfail(using_string_dtype(), reason="TODO(infer_string)") + def test_expr_with_string_with_backticks(self): + # GH 59285 + df = DataFrame(("`", "`````", "``````````"), columns=["#backticks"]) + result = df.query("'```' < `#backticks`") + expected = df["```" < df["#backticks"]] + tm.assert_frame_equal(result, expected) + + @pytest.mark.xfail(using_string_dtype(), reason="TODO(infer_string)") + def test_expr_with_string_with_backticked_substring_same_as_column_name(self): + # GH 59285 + df = DataFrame(("`", "`````", "``````````"), columns=["#backticks"]) + result = df.query("'`#backticks`' < `#backticks`") + expected = df["`#backticks`" < df["#backticks"]] + tm.assert_frame_equal(result, expected) + + @pytest.mark.parametrize( + "col1,col2,expr", + [ + ("it's", "that's", "`it's` < `that's`"), + ('it"s', 'that"s', '`it"s` < `that"s`'), + ("it's", 'that\'s "nice"', "`it's` < `that's \"nice\"`"), + ("it's", "that's #cool", "`it's` < `that's #cool` # This is a comment"), + ], + ) + def test_expr_with_column_names_with_special_characters(self, col1, col2, expr): + # GH 59285 + df = DataFrame( + [ + {col1: 1, col2: 2}, + {col1: 3, col2: 4}, + {col1: -1, col2: -2}, + {col1: -3, col2: -4}, + ] + ) + result = df.query(expr) + expected = df[df[col1] < df[col2]] + tm.assert_frame_equal(result, expected) + + @pytest.mark.xfail(using_string_dtype(), reason="TODO(infer_string)") + def test_expr_with_no_backticks(self): + # GH 59285 + df = DataFrame(("aaa", "vvv", "zzz"), columns=["column_name"]) + result = df.query("'value' < column_name") + expected = df["value" < df["column_name"]] + tm.assert_frame_equal(result, expected) + + def test_expr_with_no_quotes_and_backtick_is_unmatched(self): + # GH 59285 + df = DataFrame((1, 5, 10), columns=["column-name"]) + with pytest.raises((SyntaxError, TokenError), match="invalid syntax"): + df.query("5 < `column-name") + + def test_expr_with_no_quotes_and_backtick_is_matched(self): + # GH 59285 + df = DataFrame((1, 5, 10), columns=["column-name"]) + result = df.query("5 < `column-name`") + expected = df[5 < df["column-name"]] + tm.assert_frame_equal(result, expected) + + def test_expr_with_backtick_opened_before_quote_and_backtick_is_unmatched(self): + # GH 59285 + df = DataFrame((1, 5, 10), columns=["It's"]) + with pytest.raises( + (SyntaxError, TokenError), match="unterminated string literal" + ): + df.query("5 < `It's") + + def test_expr_with_backtick_opened_before_quote_and_backtick_is_matched(self): + # GH 59285 + df = DataFrame((1, 5, 10), columns=["It's"]) + result = df.query("5 < `It's`") + expected = df[5 < df["It's"]] + tm.assert_frame_equal(result, expected) + + def test_expr_with_quote_opened_before_backtick_and_quote_is_unmatched(self): + # GH 59285 + df = DataFrame(("aaa", "vvv", "zzz"), columns=["column-name"]) + with pytest.raises( + (SyntaxError, TokenError), match="unterminated string literal" + ): + df.query("`column-name` < 'It`s that\\'s \"quote\" #hash") + + @pytest.mark.xfail(using_string_dtype(), reason="TODO(infer_string)") + def test_expr_with_quote_opened_before_backtick_and_quote_is_matched_at_end(self): + # GH 59285 + df = DataFrame(("aaa", "vvv", "zzz"), columns=["column-name"]) + result = df.query("`column-name` < 'It`s that\\'s \"quote\" #hash'") + expected = df[df["column-name"] < 'It`s that\'s "quote" #hash'] + tm.assert_frame_equal(result, expected) + + @pytest.mark.xfail(using_string_dtype(), reason="TODO(infer_string)") + def test_expr_with_quote_opened_before_backtick_and_quote_is_matched_in_mid(self): + # GH 59285 + df = DataFrame(("aaa", "vvv", "zzz"), columns=["column-name"]) + result = df.query("'It`s that\\'s \"quote\" #hash' < `column-name`") + expected = df['It`s that\'s "quote" #hash' < df["column-name"]] + tm.assert_frame_equal(result, expected) def test_call_non_named_expression(self, df): """ From 1044cf442109953987c1a47f476dc90d286b9f0f Mon Sep 17 00:00:00 2001 From: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> Date: Tue, 20 Aug 2024 10:32:23 -1000 Subject: [PATCH 2/2] CI: Uninstall nomkl & 32 bit Interval tests (#59553) * undo numpy 2 changes? * some interval 32 bit tests working * Revert "undo numpy 2 changes?" This reverts commit 39ce2229a96406edac107fc897e807251d364e2b. * nomkl? * nomkl? * Update .github/actions/build_pandas/action.yml * grep for nomkl * xfail WASM * Reverse condition --- .github/actions/build_pandas/action.yml | 7 +++++++ pandas/tests/indexes/interval/test_interval_tree.py | 7 +++++-- pandas/tests/indexing/interval/test_interval.py | 4 ++-- pandas/tests/indexing/interval/test_interval_new.py | 4 ++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/.github/actions/build_pandas/action.yml b/.github/actions/build_pandas/action.yml index 460ae2f8594c05..6eac6fcf84f513 100644 --- a/.github/actions/build_pandas/action.yml +++ b/.github/actions/build_pandas/action.yml @@ -22,6 +22,13 @@ runs: fi shell: bash -el {0} + - name: Uninstall nomkl + run: | + if conda list nomkl | grep nomkl 1>/dev/null; then + conda remove nomkl -y + fi + shell: bash -el {0} + - name: Build Pandas run: | if [[ ${{ inputs.editable }} == "true" ]]; then diff --git a/pandas/tests/indexes/interval/test_interval_tree.py b/pandas/tests/indexes/interval/test_interval_tree.py index 49b17f8b3d40ee..df9c3b390f6601 100644 --- a/pandas/tests/indexes/interval/test_interval_tree.py +++ b/pandas/tests/indexes/interval/test_interval_tree.py @@ -4,7 +4,10 @@ import pytest from pandas._libs.interval import IntervalTree -from pandas.compat import IS64 +from pandas.compat import ( + IS64, + WASM, +) import pandas._testing as tm @@ -186,7 +189,7 @@ def test_construction_overflow(self): expected = (50 + np.iinfo(np.int64).max) / 2 assert result == expected - @pytest.mark.xfail(not IS64, reason="GH 23440") + @pytest.mark.xfail(WASM, reason="GH 23440") @pytest.mark.parametrize( "left, right, expected", [ diff --git a/pandas/tests/indexing/interval/test_interval.py b/pandas/tests/indexing/interval/test_interval.py index b72ef574753054..6bcebefa6c696e 100644 --- a/pandas/tests/indexing/interval/test_interval.py +++ b/pandas/tests/indexing/interval/test_interval.py @@ -2,7 +2,7 @@ import pytest from pandas._libs import index as libindex -from pandas.compat import IS64 +from pandas.compat import WASM import pandas as pd from pandas import ( @@ -210,7 +210,7 @@ def test_mi_intervalindex_slicing_with_scalar(self): expected = Series([1, 6, 2, 8, 7], index=expected_index, name="value") tm.assert_series_equal(result, expected) - @pytest.mark.xfail(not IS64, reason="GH 23440") + @pytest.mark.xfail(WASM, reason="GH 23440") @pytest.mark.parametrize("base", [101, 1010]) def test_reindex_behavior_with_interval_index(self, base): # GH 51826 diff --git a/pandas/tests/indexing/interval/test_interval_new.py b/pandas/tests/indexing/interval/test_interval_new.py index 4c1efe9e4f81dc..051dc7b98f2aa3 100644 --- a/pandas/tests/indexing/interval/test_interval_new.py +++ b/pandas/tests/indexing/interval/test_interval_new.py @@ -3,7 +3,7 @@ import numpy as np import pytest -from pandas.compat import IS64 +from pandas.compat import WASM from pandas import ( Index, @@ -214,7 +214,7 @@ def test_loc_getitem_missing_key_error_message( obj.loc[[4, 5, 6]] -@pytest.mark.xfail(not IS64, reason="GH 23440") +@pytest.mark.xfail(WASM, reason="GH 23440") @pytest.mark.parametrize( "intervals", [