BUG: column names with degree sign make query fail by fdion · Pull Request #42826 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation11 Commits8 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This pull request adds DEGREESIGN (°) to tokens that are parsed and escaped in a backticked column. There are no separate issue reporting this, but this is similar to the EUROSIGN issue previously addressed.
- closes #xxxx
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
Degree symbol is very common in column names for units (angle, temperature in F, C or K). This will allow query() backticked columns with degree sign in the string.
Bugfix: column names with degree sign make query fail BUG: column names with degree sign make query fail
Thanks for the pr @fdion! Would you mind adding a test that fails on master, but works with this addition? And could use a whatsnew note about this change.
Since there were no tests for the column names in query, I added a test @mzeitlin11 that covers something that worked on main (Capacitance(μF)) with a special character, and with the degree symbol (Temp(°C)) which will fail on main. All 4 tests pass with my change.
| @@ -192,6 +192,11 @@ Datetimelike |
|---|
| - Bug in :class:`DataFrame` constructor unnecessarily copying non-datetimelike 2D object arrays (:issue:`39272`) |
| - |
| Expressions |
| ^^^^^^^^^^^ |
| - Bug in :function:`create_valid_python_identifier` did not handle the degree sign in a backticked column name, such as \`Temp(°C)\`, using an expression to query a dataframe (:issue:`42826`) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually try to reference only user-facing functions in whatsnew - could you instead mention :meth:DataFrame.query (and I suppose this could also be a bug for eval too?)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. might fix eval, but I can't come up with a good scenario to test.
| @@ -2035,6 +2035,15 @@ def test_truediv_deprecated(engine, parser): |
|---|
| assert match in str(m[0].message) |
| @pytest.mark.parametrize("column", ["Temp(°C)", "Capacitance(μF)"]) |
| def test_query_token(engine, column): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a comment referencing this pr here?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and done
Since there were no tests for the column names in query, I added a test @mzeitlin11 that covers something that worked on main (
Capacitance(μF)) with a special character, and with the degree symbol (Temp(°C)) which will fail on main. All 4 tests pass with my change.
Thanks, tests look great! 2 small comments, otherwise LGTM
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small comment, ping on green.
| @@ -192,6 +192,11 @@ Datetimelike |
|---|
| - Bug in :class:`DataFrame` constructor unnecessarily copying non-datetimelike 2D object arrays (:issue:`39272`) |
| - |
| Expressions |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to indexing
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done and fixed merge conflict
feefladder pushed a commit to feefladder/pandas that referenced this pull request
