PubChemPy
PubChemPy copied to clipboard
Use Mozilla certificates in urlopen()
This suggestion arises from using PubChemPy in an application that has binaries built with pyinstaller. When this is done in a GitHub action, for some reason on MacOS there are failures at runtime associated with self-certified certificates; detail at https://github.com/molpro/iMolpro/issues/140. I have not got to the bottom of why, but the problem is solved by explicitly using the Mozilla collection of root certificates in the call to urlopen().
Perhaps this addresses https://github.com/mcs07/PubChemPy/issues/89 ?
Although it hasn't been officially merged, it effectively resolved my issue from #89 , thanks.
@mcs07 please could you consider merging this widely-needed fix so that none of us have to continue holding privately-modified copies? Thank you.
@pjknowles After a first round of edits, pubchempy was able to pass almost all the checks of the pytest test bank left by @mcs07 (see #96) when using GitHub's Ubuntu-24.04 runner and Python 3.13. Now I'm on my way to include your PR which allows to equally work with Windows 10 (and the GitHub runner of Windows 2025) and MacOS (at least by checks with the GitHub runner of macos-15, currently in my branch dev).
Because I would like to acknowledge your contribution in file docs/source/guide/contribute.rst my question to you is which shorthand should I use. Either
- |ghi| molpro <https://github.com/molpro> (Peter Knowles)
because your PR #90 originates from this fork, or
- |ghi| pjknowles <https://github.com/pjknowles> (Peter Knowles)
closer to your name?
I would like to set up an entry on PyPI reflecting this update. So far unsuccessful to get in touch with Matthew Swain, this only is going to work with a different name than pubchempy. In analogy to openbabel (last update in 2020) and openbabel-wheel (continued updates, most recently January 2025), I think pubchempy-wheel as name to PyPI could be suitable without changing the commands within the library itself.
To familiarize myself with PyPI's upload mechanism, test.pypi.org was ventured out briefly as test pad (see here) -- hence the still odd version 0.0.1 (only valid to test.pypi.org) and which (because pandas only is on the real index) doesn't resolve full. Hence the .zip below including the incomplete wheel.
Best regards
Hi @nbehrnd, apologies for the lack of maintenance on this project and thanks for your work on some much needed fixes. I'm happy to upload a new release to PyPI with your fixes under the original pubchempy name - I think that will be best to avoid confusion. Let me review and merge the various PRs and then I'll make a new release.
@pjknowles After a first round of edits, pubchempy was able to pass almost all the checks of the pytest test bank left by @mcs07 (see #96) when using GitHub's Ubuntu-24.04 runner and Python 3.13. Now I'm on my way to include your PR which allows to equally work with Windows 10 (and the GitHub runner of Windows 2025) and MacOS (at least by checks with the GitHub runner of macos-15, currently in my branch dev).
Great, thank you
Because I would like to acknowledge your contribution in file
docs/source/guide/contribute.rstmy question to you is which shorthand should I use. Either- |ghi| molpro <https://github.com/molpro> (Peter Knowles)because your PR #90 originates from this fork, or
- |ghi| pjknowles <https://github.com/pjknowles> (Peter Knowles)closer to your name?
Yes, pjknowles is me, and molpro is just an organisational namespace
@mcs07
I welcome your return to pubchempy and multiple PRs entering the default branch. As you see, because of size and complexity of the project, my approach was a little bit different, i.e. first to use your tests as a handrail eventually automatically launched (as a fix, a potential pubchempy 1.0.5), and subsequently consider and include PRs filed (e.g., #41) for a pubchempy 1.1.0.
The tentative name of pubchempy-wheel defined in the pyproject.toml file would affect only the name of the wheel built and the record on PyPI; no change to the name of a repository on GitHub, nor to a command provided by the Python script. By now, it appears this is not necessary.
Please note, your merges yesterday up to and including 2ba3fc7 address changes in Python syntax. Without introduction of certifi this PR #90 is about -- both in terms of the requirements* as well as import and use in the script -- pubchempy still/again does not work in contemporary versions of Windows. Which is the reason why I filed PR #98.
* With a pyproject.toml file present, one equally should be able to resolve the requirements of the cloned project in a virtual environment e.g., by
pip install .
without need of a separate requirements file.
@pjknowles Yesterday, Matthew merged a couple of PRs into the project's default branch. I speculate, maybe within the next 15 days the remaining PRs equally will find their way -- as such, or with edits -- into the project. The documentation file was updated. Let us see.
@mcs07 With the admission of multiple PRs up and including commit 2ba3fc7 on April 29, 2025, the backlog of pubchempy was reduced. There still are some PRs in the queue. Indeed adoption of PR #65 allows to update the local documentation by a local call of Sphinx' assistance, however 1) the one on readthedocs.io reflects the state of 2014 (e.g., print compound.cid instead of now print(compound.cid)). And 2) the entry on PyPI is about the state of 2017.
Were it possible to update e.g., to version 1.0.5 throughout the project, including readthedocs and PyPI; and continue the gradual review of pending PRs?
@mcs07 With the admission of multiple PRs up and including commit 2ba3fc7 on April 29, 2025, the backlog of pubchempy was reduced. There still are some PRs in the queue. Indeed adoption of PR #65 allows to update the local documentation by a local call of Sphinx' assistance, however 1) the one on readthedocs.io reflects the state of 2014 (e.g.,
print compound.cidinstead of nowprint(compound.cid)). And 2) the entry on PyPI is about the state of 2017.Were it possible to update e.g., to version 1.0.5 throughout the project, including readthedocs and PyPI; and continue the gradual review of pending PRs?
I would really appreciate it if we get to the point where this PR was accepted, and, I guess through version tagging, available on conda-forge. It would enable me to remove a work-around in the build of another project. Happy to discuss if there is a suspicion that the change to using the Mozilla certificates might cause a problem I hadn't anticipated. Also I would be relaxed about not making it the default, but only on the passing of a keyword argument through to request(), if that were felt to be necessary.
Incorporated into #118 with some additional changes and merged.
Thanks for the contribution Peter!