Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate setup configuration to setup.cfg #31

Merged
merged 6 commits into from
May 12, 2021

Conversation

DragaDoncila
Copy link
Collaborator

  • Moved configuration options into setup.cfg.
  • For editable installs, kept use_scm in setup.py.
  • setup.cfg doesn't support a file: option for the install_requires keyword, and it doesn't look like that'll be coming anytime soon, based on the discussion in this issue. So for our purposes I've kept the requirements reading in setup.py

I built a plugin locally and it worked as expected. I tried running tests locally but running the created plugin's tests failed with

E           TypeError: Failed expected string as 'msg' parameter, got 'CalledProcessError' instead.
E           Perhaps you meant to use a mark?

I saw the same behaviour on master though so I think it must be something I'm doing wrong.

Addresses #26.

@tlambert03
Copy link
Contributor

setup.cfg doesn't support a file: option for the install_requires keyword, and it doesn't look like that'll be coming anytime soon, based on the discussion in this issue. So for our purposes I've kept the requirements reading in setup.py

let's move away from reading requirements.txt into the setup.cfg... and do it the other way around instead:

# requirements.txt

-e .

@DragaDoncila
Copy link
Collaborator Author

Umm, I'm confused how that would work.
I thought we needed to read from requirements.txt because we don't know what the minimal installation requirements are, and if we don't move them into setup.py then users would have to install using pip install -r.

But are you saying that there's a relationship between the editable install and the requirements file? What does adding -e . to the requirements file do? And, does that mean users would have to add their dependencies into setup.cfg themselves, rather than in requirements.txt where we're telling them to put them now?

@tlambert03
Copy link
Contributor

highly recommend this article: https://caremad.io/posts/2013/07/setup-vs-requirement/

requirements.txt is only there for the ability to do pip install -r requirements.txt which (to my understanding) has come to be an expected way to set up an environment, particular for non-redistributable things... like "i go this set of things to work, and here are the requirements". install_requires (in setup.py, setup.cfg), on the other hand, is the canonical way of specifying what an installable package/library needs to run.

does that mean users would have to add their dependencies into setup.cfg themselves, rather than in requirements.txt where we're telling them to put them now?

exactly. that's the "right" place for a package to declare it's dependencies... and the "right" way to install a package after cloning it is probably pip install -e . ... so, adding -e . to requirements.txt is just a "courtesy" that lets people used to doing pip install -r requirements.txt keep their muscle memory. (but... we could also just get rid of it entirely)

@DragaDoncila
Copy link
Collaborator Author

@tlambert03 thanks for the details - I know they're very newbie questions. If I've understood you correctly, then these latest two commits should do it.

{{cookiecutter.plugin_name}}/setup.cfg Outdated Show resolved Hide resolved
{{cookiecutter.plugin_name}}/setup.cfg Outdated Show resolved Hide resolved
{{cookiecutter.plugin_name}}/setup.cfg Outdated Show resolved Hide resolved
@jni jni merged commit c7d5b8a into napari:master May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants