Skip to content
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

BUG: to_xml with index=False and offset input index #42464

Merged
merged 2 commits into from
Jul 12, 2021

Conversation

stephan-hesselmann-by
Copy link
Contributor

@stephan-hesselmann-by stephan-hesselmann-by commented Jul 9, 2021

Fixes #42458

It was assumed that the index contains the element 0. This led to a
defect when the index of the input Dataframe has an offset, which is a
common use case when streaming Dataframes via generators.

This fix consists of not relying on accessing the 0 element of
frame_dicts.

Fixes pandas-dev#42458

It was assumed that the index contains the element `0`. This led to a
defect when the index of the input Dataframe has an offset, which is a
common use case when streaming Dataframes via generators.

This fix consists of not relying on accessing the `0` element of
`frame_dicts`.
@jreback
Copy link
Contributor

jreback commented Jul 12, 2021

cc @ParfaitG if you can have a look here

@jreback jreback added the IO XML read_xml, to_xml label Jul 12, 2021
@jreback jreback added this to the 1.3.1 milestone Jul 12, 2021
@jreback jreback added the Bug label Jul 12, 2021
Copy link
Contributor

@ParfaitG ParfaitG left a comment

Choose a reason for hiding this comment

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

Good catch on this issue and good work with this fix, @stephan-hesselmann-by! I have minor comments.

pandas/io/formats/xml.py Outdated Show resolved Hide resolved
pandas/io/formats/xml.py Outdated Show resolved Hide resolved
pandas/tests/io/xml/test_to_xml.py Outdated Show resolved Hide resolved
@@ -290,6 +293,42 @@ def test_index_false_rename_row_root(datapath, parser):
assert output == expected


def test_index_false_with_offset_input_index(parser):
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure we annotate reason for test even on bug fixes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can remove the docstring if it goes against coding guidelines for the project. But personally I like to give context to tests like these.

Copy link
Contributor

Choose a reason for hiding this comment

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

we normally don't add this, just a comment to the issue. but no harm as you have written it.

Copy link
Contributor

@ParfaitG ParfaitG left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, @stephan-hesselmann-by!

@jreback jreback merged commit d60e687 into pandas-dev:master Jul 12, 2021
@jreback
Copy link
Contributor

jreback commented Jul 12, 2021

thanks @stephan-hesselmann-by for the patch and @ParfaitG for the review!

@jreback
Copy link
Contributor

jreback commented Jul 12, 2021

@meeseeksdev backport 1.3.x

@lumberbot-app
Copy link

lumberbot-app bot commented Jul 12, 2021

Something went wrong ... Please have a look at my logs.

simonjayhawkins pushed a commit that referenced this pull request Jul 13, 2021
… index (#42513)

Co-authored-by: Stephan Heßelmann <60873230+stephan-hesselmann-by@users.noreply.github.com>
@stephan-hesselmann-by stephan-hesselmann-by deleted the fix-xml branch July 30, 2021 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug IO XML read_xml, to_xml
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: to_xml raises KeyError for index=False when the index does not start at zero
3 participants