[Dev] Remove `mysql` dependency from `ext.cod` and fix S608

materialsproject / pymatgen

Python Materials Genomics (pymatgen) is a robust materials analysis code that defines classes for structures and molecules with support for many electronic structure codes. It powers the Materials Project.

https://pymatgen.org

Other

1.52k stars 864 forks source link

[Dev] Remove `mysql` dependency from `ext.cod` and fix S608 #4016

Closed DanielYang59 closed 1 month ago

DanielYang59 commented 2 months ago

Resolve following mysql dependency issue: https://github.com/materialsproject/pymatgen/blob/41e4c69911f2b724734e0917c436ee5a1de63091/src/pymatgen/ext/cod.py#L70
Also need to fix two S608 violations (SQL injection): https://github.com/materialsproject/pymatgen/blob/41e4c69911f2b724734e0917c436ee5a1de63091/src/pymatgen/ext/cod.py#L73 https://github.com/materialsproject/pymatgen/blob/41e4c69911f2b724734e0917c436ee5a1de63091/src/pymatgen/ext/cod.py#L109

Help needed.

janosh commented 2 months ago

Resolve following mysql dependency issue:

i think mysql can be replaced with https://pypi.org/project/mysqlclient (based on the advice here)

shyuep commented 1 month ago

Why bother? This is a small feature within pymatgen and we probably can count on a few fingers the number of people who use it. Those who use it can install mysql. In fact, the onus should be on COD to offer a REST API rather than an SQL one. Pls focus efforts on more impactful changes.

DanielYang59 commented 1 month ago

This is a small feature within pymatgen and we probably can count on a few fingers the number of people who use it. Those who use it can install mysql.

That's true, I posted this in case someone who need to use SQL might be interested to take over and fix this?

Pls focus efforts on more impactful changes.

That's the reason I opened this instead of fixing it myself. Can we reopen this? This looks like a good first issue if anyone is interested in getting involved in open source.