Skip to content

DEPR: Remove literal string input for read_xml #53809

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.

Sign up for GitHub

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 23 commits into from
Jul 11, 2023
Merged

DEPR: Remove literal string input for read_xml #53809

Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f347e8e
Updating documentation and adding deprecation logic for read_xml.
rmhowe425 Jun 22, 2023
296b45a
Fixing documentation issue and adding unit test
rmhowe425 Jun 23, 2023
69cdc1a
Updating unit tests and documentation.
rmhowe425 Jun 23, 2023
83a9177
Merge branch 'main' into dev/depr/literal-str-read_xml
rmhowe425 Jun 23, 2023
0f0f38b
Fixing unit tests and documentation issues
rmhowe425 Jun 24, 2023
2c848ac
Fixing unit tests and documentation issues
rmhowe425 Jun 24, 2023
b8a582c
Fixing unit tests and documentation issues
rmhowe425 Jun 24, 2023
92bc6fa
Fixing import error in documentation
rmhowe425 Jun 24, 2023
8bbd7c4
Updated deprecation logic per reviewer recommendations.
rmhowe425 Jun 26, 2023
5aece78
Updating deprecation logic and documentation per reviewer recommendat…
rmhowe425 Jun 26, 2023
6f15924
Fixing logic error
rmhowe425 Jun 26, 2023
00f7b15
Fixing implementation per reviewer recommendations.
rmhowe425 Jun 27, 2023
20e7ef2
Updating implementation per reviewer recommendations.
rmhowe425 Jun 27, 2023
526c224
Cleaning up the deprecation logic a bit.
rmhowe425 Jun 27, 2023
9dfa18d
Merge branch 'main' into dev/depr/literal-str-read_xml
rmhowe425 Jun 27, 2023
65f88b9
Updating implementation per reviewer recommendations.
rmhowe425 Jun 27, 2023
ec28efa
Merge branch 'main' into dev/depr/literal-str-read_xml
rmhowe425 Jun 28, 2023
2c58638
Merge branch 'main' into dev/depr/literal-str-read_xml
rmhowe425 Jun 29, 2023
e08f4e0
Merge branch 'main' into dev/depr/literal-str-read_xml
rmhowe425 Jun 30, 2023
ba1edd6
Merge branch 'main' into dev/depr/literal-str-read_xml
rmhowe425 Jul 9, 2023
b7e1fb6
Updating unit tests
rmhowe425 Jul 9, 2023
14d2cb1
Fixing discrepancy in doc string.
rmhowe425 Jul 9, 2023
c215a94
Updating implementation based on reviewer recommendations.
rmhowe425 Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pandas/io/xml.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1127,7 +1127,7 @@ def read_xml(
"""
check_dtype_backend(dtype_backend)

if not any(
if isinstance(path_or_buffer, str) and not any(
Copy link
Contributor

Choose a reason for hiding this comment

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

Simply for organization and not a must, can we move this warning logic to the next called method _parse written just before this main read_xml call? This moves it away from the docs lines. Also, in deprecated warning in docs, consider changing html to xml, two different types of documents.

Also, other unit tests other than your new one which reads literal strings are failing with this warning. You may have to check for Pathlike objects as well as string. Be sure to run pytest locally before pushing commit!

Copy link
Contributor Author

@rmhowe425 rmhowe425 Jul 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@mroeschke the documentation discrepancy is fixed here. Also, unit tests are passing and CI tests are green. Pathlike objects are already being checked in my conditional statement in xml.py

Not sure how you feel about moving the warning logic to _parse. This isn't something that we did in similar PRs. Not sure it matters?

Otherwise, I think this PR is ready for inspection.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed this warning would be better suited in _parse as well.

[
is_file_like(path_or_buffer),
file_exists(path_or_buffer),
Expand Down