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

Fix multi dimensional arrays with FlattenedStorage.to_pandas #1335

Merged
merged 8 commits into from
Jul 17, 2024
Merged

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Feb 14, 2024

No description provided.

@pmrv pmrv added the bug Something isn't working label Feb 14, 2024
Copy link
Member

@jan-janssen jan-janssen left a comment

Choose a reason for hiding this comment

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

To guarantee the stability of these kinds of conversions it would be great to add tests.

@jan-janssen jan-janssen marked this pull request as draft February 14, 2024 14:34
Copy link

stale bot commented Mar 17, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 17, 2024
@stale stale bot removed the stale label Jul 16, 2024
@pmrv pmrv marked this pull request as ready for review July 16, 2024 16:01
@pmrv pmrv requested a review from jan-janssen July 16, 2024 16:03
@jan-janssen
Copy link
Member

@pmrv Can you merge main into your branch? Especially as the directory structure for the tests changed?

@pmrv
Copy link
Contributor Author

pmrv commented Jul 16, 2024

@pmrv Can you merge main into your branch? Especially as the directory structure for the tests changed?

Should be fixed now.

pyiron_base/_tests.py Outdated Show resolved Hide resolved
Comment on lines 966 to 972
try:
dfe = store.to_pandas(explode=True)
except:
# Regression test
self.fail(
"to_pandas must not fail when multidimensional arrays are present!"
)
Copy link
Member

Choose a reason for hiding this comment

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

I am a bit confused about the try except part. I agree that it gives a nicer error message but at the same time it is not as readable.

pyiron_base/_tests.py Outdated Show resolved Hide resolved
Copy link
Member

@jan-janssen jan-janssen left a comment

Choose a reason for hiding this comment

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

Looks good to me

@pmrv pmrv merged commit 7bc4bde into main Jul 17, 2024
27 checks passed
@pmrv pmrv deleted the topandas branch July 17, 2024 08:34
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.

2 participants