search

jhrmnn / pyberny

Molecular structure optimizer
Mozilla Public License 2.0
110 stars 21 forks source link

Removed dev dependencies from dependencies #37

Closed coltonbh closed 1 year ago

coltonbh commented 1 year ago

Hi @jhrmnn . Thanks for your work on berny!

I've updated your pyproject.toml file to remove developer-only dependencies from your main dependencies. Having pytest, sphinx, toml, etc. in your dependencies adds them to builds when one only wants a library installation of the package (for example when installing from PyPi). This helps avoid dependency conflicts and keeps installs clean and lightweight.

I'd also recommend adding a poetry.lock file to your repo so that developer builds are reproducible. I'm omitted that from this PR but let me know if you'd like it added and I'll make a new PR with the lock file I generated from the updated pyproject.toml.

If you accept this PR would you be up for releasing a version 0.6.4 on PyPi with the update? Would be great to have a slimmed down berny with only user dependencies available :)

Thanks so much!

jhrmnn commented 1 year ago

Hi! And thanks for the PR. I’m trying to understand the motivation here.

The dependencies you modify are already listed as optional. So unless you explicitely activate them via extras, they won’t install.

As for poetry.lock, the Poetry documentation says that it’s not necessary to commit it for library projects, and I’d like to stay with that. There is no canonical set of dependency versions that should be shared among developers.

https://python-poetry.org/docs/basic-usage/#commit-your-poetrylock-file-to-version-control

coltonbh commented 1 year ago

You're right re: optional! I didn't even catch that.

I was building a few projects recently where libraries had mixed dev dependencies into package dependencies and I wanted to clean up some of that up. I checked pyberny and noticed these dev packages under the standard dependencies header so made a quick PR to fix it. The motivation was to clean up some of my builds by patching packages that had unnecessarily bundled dev packages into their installs. You're right that for pyberny they don't actually install under normal conditions due to the optional setting. I missed that.

If it were me I'd probably move them to dev-dependencies and make them not optional since they are explicit dev-deps. But my primary concern of pyberny installing dev packages into my end users applications is resolved. I've updated the PR to reflect this; feel free to close if this is unimportant to you.

Re: pyproject.lock I like having reproducible builds for developers, even though the library install will be more flexible, but that's a matter of taste--not required ;P

Thanks for the quick response and for the package! Much appreciated :)

jhrmnn commented 1 year ago

Ok, happy this is resolved now :) I actually like to not pin the dependencies for a library, because it sometimes uncovers issues with dependencies. So I’ll keep it as is. But thanks for your effort!