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

pymatgen>=2024.8.8 is updated in the .toml file after fixing of ion parsing bug #169

Closed
wants to merge 0 commits into from

Conversation

abhardwaj73
Copy link
Contributor

Summary

Major changes:

  • fix 1: pymatgen>=2024.8.8 is updated in the .toml file

@rkingsbury rkingsbury added the dependencies Pull requests that update a dependency file label Aug 9, 2024
@rkingsbury
Copy link
Member

Thanks @abhardwaj73 ! I wasn't expecting the test failures; I need to figure out if there is a bug in pymatgen related to triiodide ion because I see 'I3[-3]' in the test output, whereas it should be 'I3[-1]' Same with C2O4[-0.5]

@rkingsbury
Copy link
Member

rkingsbury commented Aug 9, 2024

Fixes #158

@rkingsbury
Copy link
Member

Ok yeah I'll need to fix this one in pymatgen

>>> Ion.from_formula('CO2-').reduced_formula
'C2O4[-0.5]'
>>> Ion.from_formula('CO2--').reduced_formula
'C2O4[-1]'
>>>

Tri-iodide seems OK, but the logic is incorrectly parsing I[-1]

>>> Ion.from_formula('I3-').get_reduced_formula_and_factor()
('I3', 1.0)
>>> Ion.from_formula('I-').get_reduced_formula_and_factor()
('I3', 0.3333333333333333)

@rkingsbury
Copy link
Member

OK, pymatgen PR is open to fix the upstream formula parsing bugs. Let's pause this PR until the next pymatgen release.

@rkingsbury
Copy link
Member

OK, pymatgen PR is open to fix the upstream formula parsing bugs. Let's pause this PR until the next pymatgen release.

The release containing the bugfix is out - https://github.com/materialsproject/pymatgen/releases/tag/v2024.9.10

@abhardwaj73
Copy link
Contributor Author

abhardwaj73 commented Sep 10, 2024

I can't seem to update the pymatgen version in the pyproject.toml file. I see the warning "This branch has conflicts that must be resolved" with the base file for the pymatgen version. Maybe only you can update it @rkingsbury ?

@rkingsbury
Copy link
Member

I can't seem to update the pymatgen version in the pyproject.toml file. I see the warning "This branch has conflicts that must be resolved" with the base file for the pymatgen version. Maybe only you can update it @rkingsbury ?

Hmmm it sounds like you have a merge conflict. On your local branch, make sure to pull from the main branch
git pull origin main

and then you should be able to resolve the conflict in your local editor and commit the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants