You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "AlenkaF (via GitHub)" <gi...@apache.org> on 2025/06/23 14:27:26 UTC
[PR] GH-46179: Bump index level once if pandas df already contains __index_level_i__ column [arrow]
AlenkaF opened a new pull request, #46884:
URL: https://github.com/apache/arrow/pull/46884
### Rationale for this change
### What changes are included in this PR?
### Are these changes tested?
### Are there any user-facing changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd merged PR #46884:
URL: https://github.com/apache/arrow/pull/46884
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #46884:
URL: https://github.com/apache/arrow/pull/46884#issuecomment-4649916693
cc @raulcd if you have some time for review. No rush!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #46884:
URL: https://github.com/apache/arrow/pull/46884#issuecomment-4709748068
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit a81e6c6015db4ab024bfd3a2d8666bde014ba6f1.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/81464391040) has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #46884:
URL: https://github.com/apache/arrow/pull/46884#issuecomment-4679748326
Dev failure does not seem to be connected to the changes in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on code in PR #46884:
URL: https://github.com/apache/arrow/pull/46884#discussion_r3401574358
##########
python/pyarrow/tests/test_pandas.py:
##########
@@ -421,8 +421,18 @@ def test_index_level_name_bump(self):
)
_check_pandas_roundtrip(df, preserve_index=True)
- t = pa.Table.from_pandas(df, preserve_index=True)
- assert t.schema.names == ['col', '__index_level_0__', '__index_level_1__']
+ # Explicit test
+ t = pa.table(df)
+ expected_schema = pa.schema([
+ ("col", pa.int64()),
+ ("__index_level_0__", pa.int64()),
+ ("__index_level_1__", pa.int64())
+ ])
+ assert t.schema.equals(expected_schema)
+
+ df2 = t.to_pandas()
+ assert df2.index.equals(pd.Index([10, 20, 30]))
Review Comment:
yes, I think I was going for this! Thanks @AlenkaF
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #46884:
URL: https://github.com/apache/arrow/pull/46884#discussion_r3302641177
##########
python/pyarrow/pandas_compat.py:
##########
@@ -378,7 +378,10 @@ def _index_level_name(index, i, column_names):
if index.name is not None and index.name not in column_names:
return _column_name_to_strings(index.name)
else:
- return f'__index_level_{i:d}__'
+ j = i
+ while f'__index_level_{j:d}__' in column_names:
+ j += 1
+ return f'__index_level_{j:d}__'
Review Comment:
Isn't schema based conversion already buggy without this change when it comes to the index levels? It probably silently ignores the duplicated level `0` currently?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #46884:
URL: https://github.com/apache/arrow/pull/46884#issuecomment-4542835912
@jorisvandenbossche what do you think of the proposed change in this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #46884:
URL: https://github.com/apache/arrow/pull/46884#issuecomment-4679515480
Thanks for the review @raulcd!
I have added an explicit test: https://github.com/apache/arrow/pull/46884/changes/c5f17705c5ea4978fff91b48d9889b88e029df84. Is this what you has in mind?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46884:
URL: https://github.com/apache/arrow/pull/46884#issuecomment-2996719353
:warning: GitHub issue #46179 **has been automatically assigned in GitHub** to PR creator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #46884:
URL: https://github.com/apache/arrow/pull/46884#discussion_r3302735537
##########
python/pyarrow/pandas_compat.py:
##########
@@ -378,7 +378,10 @@ def _index_level_name(index, i, column_names):
if index.name is not None and index.name not in column_names:
return _column_name_to_strings(index.name)
else:
- return f'__index_level_{i:d}__'
+ j = i
+ while f'__index_level_{j:d}__' in column_names:
+ j += 1
+ return f'__index_level_{j:d}__'
Review Comment:
OK, getting used to this :) Copilot can't answer. Well, I think the change suggested can be a possible follow-up if we see this would be needed. But I do not think it is in the scope of this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-46179: [Python] Bump index level once if pandas df already contains __index_level_i__ column [arrow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #46884:
URL: https://github.com/apache/arrow/pull/46884#discussion_r3298718286
##########
python/pyarrow/pandas_compat.py:
##########
@@ -378,7 +378,10 @@ def _index_level_name(index, i, column_names):
if index.name is not None and index.name not in column_names:
return _column_name_to_strings(index.name)
else:
- return f'__index_level_{i:d}__'
+ j = i
+ while f'__index_level_{j:d}__' in column_names:
+ j += 1
+ return f'__index_level_{j:d}__'
Review Comment:
Bumping the generated index column name changes the meaning of the numeric suffix (it no longer matches the index level position). Later in this module, `_get_index_level()` assumes that any `__index_level_{n}__` name maps to `df.index.get_level_values(n)`. With this change, a dataframe that already has `__index_level_0__` will generate `__index_level_1__` for the first unnamed index level, but `_get_index_level(df, '__index_level_1__')` would incorrectly select level 1 (or raise for a single-level index) when a user passes an explicit schema. Consider updating `_get_index_level()` to resolve generated index names using the same collision-avoidance logic as `_index_level_name` (based on `df.columns` and already-assigned index names), so schema-based conversions keep working.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org