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

Test for type conservation #508

Merged
merged 2 commits into from
Nov 10, 2021
Merged

Test for type conservation #508

merged 2 commits into from
Nov 10, 2021

Conversation

niklassiemer
Copy link
Member

@niklassiemer niklassiemer commented Nov 10, 2021

This should cause pyiron/pyiron_atomistics#409 to pop up on our pyiron_base unittests already instead of in the compatibility with pyiron_atomistics integration tests.

Comment on lines 112 to 117
all(
np.equal(
content_dict["key_2"],
np.array([1, 2, 3, 4, 5, 6]),
)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
all(
np.equal(
content_dict["key_2"],
np.array([1, 2, 3, 4, 5, 6]),
)
)
np.array_equal(
content_dict["key_2"],
np.array([1, 2, 3, 4, 5, 6]),
)

Copy link
Contributor

Choose a reason for hiding this comment

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

That pattern comes up a few times, but array_equal should be correct for all of them.

Copy link
Contributor

@pmrv pmrv left a comment

Choose a reason for hiding this comment

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

lgtm, except for the array_equal nit.

Comment on lines 112 to 117
all(
np.equal(
content_dict["key_2"],
np.array([1, 2, 3, 4, 5, 6]),
)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

That pattern comes up a few times, but array_equal should be correct for all of them.

Co-authored-by: Marvin Poul <poul@mpie.de>
@niklassiemer niklassiemer merged commit e8ad226 into master Nov 10, 2021
@delete-merged-branch delete-merged-branch bot deleted the test_fileHDFio branch November 10, 2021 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants