Fix Incar check_params for Union type

[DanielYang59]

Summary

• Fix Incar check_params for Union type, to fix Vasp Incar check_params() can't handle Union types #3957, changed incar_parameters.json of Union type to use isinstance syntax directly. Otherwise Python 3.9 Windows don't support type checking with isinstance(data, Union[str, float]).

  Would appreciate it if anyone has smarted way to resolve this!

• Bump palettable version to remove dependency of setuptools Write colorbrewer palettes to .py file and remove setuptools dependency jiffyclub/palettable#46

[wladerer]

This might be a silly question, but is changing the "type" parameter in the json file possible? The following evaluates to typing.Union

type(eval('bool | str'))

Example:

>>> isinstance(True, eval('bool | str'))
True
>>> isinstance("string thing", eval('bool | str'))
True
>>> isinstance(55, eval('bool | str'))
False

[DanielYang59]

That's actually an ingenious solution IMO, now that using of eval is not risky in this scenario.

However the only pitfall being that the alias of Union[x, y] as x | y is introduced in Python 3.10, and as the min supported version for pymatgen is 3.9 at this moment, we still cannot implement that change. But still I think it's a cleaner solution than mine, perhaps we could import Union for now until we move to Python 3.10+.

[DanielYang59]

@janosh Can you please review this? Thanks. I believe the CI failing is unrelated to this PR.

[janosh]

I believe the CI failing is unrelated to this PR.

i think the issue is due to installing the latest torch in test.yml while matGL requires torch=2.2.1 or older

pymatgen/.github/workflows/test.yml

Line 80 in 633e7bd

pip install torch --upgrade

can you try pinning to torch 2.2.1 in CI with a TODO to remove pin later?

[DanielYang59]

Yes sure. Let me have a try.

[DanielYang59]

Looks like matgl has removed the torch version limitation, but still we need to wait for a new release. materialsvirtuallab/matgl@d0bf202

[janosh]

afaik dgl isn't compatible with torch==2.3 yet, let alone 2.4 so don't think the matgl up pin should have been removed. i was advocating for a down pin in another thread. either way, let's revert the temp torch==2.2.1 pin here and just skip the matGL tests for now to keep things moving

[DanielYang59]

Sure! Thanks for the input, and I know nothing about matgl :)

[janosh]

thanks a lot @DanielYang59! 👍

[DanielYang59]

Thanks for reviewing!