Fix compatibility with changes in IOData (prepare_segmented)#196
Fix compatibility with changes in IOData (prepare_segmented)#196leila-pujal merged 1 commit intotheochem:masterfrom
Conversation
Fixes theochem#195 The required function is imported locally from the iodata library because the wrapper module also contains a wrapper for pyscf. When the import is made at the top and IOData is not installed, it would also become impossible to use the pyscf wrapper. (You may want to split the wrapper module into one per package to avoid local imports, but this is not critical.) This commit includes a few side effects: - Ruff fixes were required in wrappers.py to make the commit. - .envrc is added to .gitignore. I used direnv for managing environments
leila-pujal
left a comment
leila-pujal
left a comment
There was a problem hiding this comment.
Hi @tovrstra, thanks a lot for your quick PR to fix issue #195. I checked your new code locally with the latest IOData with one of my scripts and it works. All tests that use from_iodata pass as well. You may be right and we should split pyscf and iodata wrappers now that we have this import specific to IOData. I think maybe because from_pyscf is not a big function they decided to put both into one module and of course, they were not importing any specific functionality. I will open an issue as an enhancement so we can keep track of this suggestion. Related to your side effects: The Ruff changes match the ones I did in wrappers.py in #193 . I will merge this PR and update #193 so no problem. Thanks for adding the .envrc to .gitignore . I am not sure how new direnv is but we should keep track better of new tools for managing enviromental variables and add them to .gitingnore. I will soon deal with PR #187 and #193. I will merge this at the end of the day to give more time to Canadian people if they want to review it as well. Thanks again!
|
You're welcome. Thanks for the review. I'll let you do the merging, so you can decide in which order to deal with the PRs. |
2 participants
The required function is imported locally from the iodata library because the wrapper module also contains a wrapper for pyscf. When the import is made at the top and IOData is not installed, it would also become impossible to use the pyscf wrapper. (You may want to split the wrapper module into one per package to avoid local imports, but this is not critical.)
This commit includes a few side effects:
Checklist
Add tests for each unit of code added (e.g. function, class)Update documentationType of Changes
Related
Fixes #195