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 maximum element failure #921

Merged
merged 1 commit into from Feb 6, 2024
Merged

Fix maximum element failure #921

merged 1 commit into from Feb 6, 2024

Conversation

JaGeo
Copy link
Contributor

@JaGeo JaGeo commented Feb 6, 2024

Should fix #920 by introducing a maximum element number. Og is probably enough.

@JaGeo
Copy link
Contributor Author

JaGeo commented Feb 6, 2024

@ml-evs Any ideas where the failures could come from? To dig into this, I would need to fix the installation locally first as well.

Okay, I see... there are simply tons of failing tests 😅 ...

@ml-evs
Copy link
Collaborator

ml-evs commented Feb 6, 2024

Hi @JaGeo, the changes in this PR look fine to me, the failing tests are simply from the lack of pandas 2.0 support (and the rest are passing fine for me locally). If you did want to verify this yourself, you can install the pinned deps in the various requirements files in this repo rather than using the loose pyproject versions.

Copy link
Collaborator

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Thanks @JaGeo!

@ml-evs ml-evs merged commit f810184 into hackingmaterials:main Feb 6, 2024
1 of 4 checks passed
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.

Simple composition-based featurization fails due to an upgrade in pymatgen
2 participants