Skip to content

fix: batch Vision GUID number columns - #492

Open
be-student wants to merge 3 commits into
PowerGridModel:mainfrom
be-student:codex/468-excel-fragmentation
Open

fix: batch Vision GUID number columns#492
be-student wants to merge 3 commits into
PowerGridModel:mainfrom
be-student:codex/468-excel-fragmentation

Conversation

@be-student

Copy link
Copy Markdown

Fixes #468.

Builds all generated Number columns in one concat operation instead of repeatedly inserting them. This removes pandas fragmentation warnings and preserves each GUID/Number pair's order, including MultiIndex inputs and existing Number columns.

Validation: 1,444 tests pass; focused tests, Ruff, MyPy, REUSE, and full pre-commit pass.

I, eunwoo song <presentsong@naver.com>, hereby add my Signed-off-by to this commit: 1011f22

Signed-off-by: eunwoo song <presentsong@naver.com>
I, eunwoo song <presentsong@naver.com>, hereby add my Signed-off-by to this commit: 1011f22

Signed-off-by: eunwoo song <presentsong@naver.com>
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo added the bug Something isn't working label Sep 7, 2026
@Jerry-Jinfeng-Guo

Copy link
Copy Markdown
Member

Hi @be-student , thank you for creating this PR! We will review the PR. If necessary, we may rebase it to a different branch.

number_values = guid_values.apply(self._uuid_cvtr.query)
if new_column_name in first_level:
number_column_pos = first_level.tolist().index(new_column_name)
data.iloc[:, number_column_pos] = number_values

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

iloc assignment keeps the existing column dtype, while the previous column assignment replaced it. With an existing object typed Number column, the generated integer values remain object typed, which can affect downstream numeric handling. I reproduced this with pandas 2.3.3. Could this use data.isetitem(number_column_pos, number_values) and add a regression assertion for the resulting dtype? This still updates by position for MultiIndex columns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[IMPROVEMENT] fix warnings around performance issues

3 participants