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

Investigate qtpy dependency #253

Open
ZLLentz opened this issue Feb 11, 2021 · 6 comments
Open

Investigate qtpy dependency #253

ZLLentz opened this issue Feb 11, 2021 · 6 comments

Comments

@ZLLentz
Copy link
Member

ZLLentz commented Feb 11, 2021

  • This is listed as required for the test, in the conda-recipe, which is just an import of pytmc, while not being listed in the requirements for the package.
  • It is listed in dev-requirements.txt but not in requirements.txt.
  • It is used in bin/types.py, which is a debugging interface, but also a command-line option for the cli entry point.
@klauer
Copy link
Contributor

klauer commented Feb 11, 2021

It's considered "optional". If qtpy is unavailable, pytmc types will similarly be unavailable (with a message in pytmc --help indicating the reason for it).

There are hooks in the conda recipe to specify optional dependencies. We have no specific version requirement here, but perhaps it'd be worth listing in that section regardless.

@ZLLentz
Copy link
Member Author

ZLLentz commented Feb 11, 2021

Ok, that makes sense. So in general this doesn't even belong in the testing section of the conda recipe? And instead belongs in optional dependencies, explicitly?

@klauer
Copy link
Contributor

klauer commented Feb 11, 2021

Well, we do actually run tests with it. If we're going to start using the test dependency section in the conda recipes (*) then it should be listed there. Otherwise, optional deps for now.

(*) I think we should consider this at some point - I either mentioned that in chat or opened an issue, I can't recall!

@ZLLentz
Copy link
Member Author

ZLLentz commented Feb 11, 2021

Right, you opened an issue somewhere. I was more referring to the fact that it's listed as a requirement for the tests, and since the test is just an import, it implies that it is required to do the import, which logically makes it a non-optional dependency.

@klauer
Copy link
Contributor

klauer commented Feb 11, 2021

Oh... oh... I'm a little slow/distracted this morning - apologies. I should just wait to comment on these.

@ZLLentz
Copy link
Member Author

ZLLentz commented Feb 11, 2021

Treat this issue as "Zach got confused while reviewing the conda-forge PR because of this dependency"

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

No branches or pull requests

2 participants