-
Notifications
You must be signed in to change notification settings - Fork 43
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
Added missing dependencies to requirements.txt #2127
Conversation
I think we should add @wpotrzebowski to the review since he was the one last working on fine tuning requirements.txt. Just in case there was something special we are missing here? |
I meant to. |
@wpotrzebowski I've removed the OS dependency - can you check I used the right versions (I picked the windows/linux associated ones) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need "pyqt5-tools" if we already have PyQt5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
importlib_resources seems to refer to importlib-resources i.e. does it appear twice on the list?
Probablty not as a part of this pull request but it may be worth to set up GH actions that "tests" requirements.txt file when change is made in it. By testing I mean pip installing on workers. |
importlib_resources seems to refer to importlib-resources i.e. does it appear twice on the list? They're for different python versions - though as we discussed recently, there is no need to support python < 3.7, so the former is not really needed. |
Could also set up tests to ensure that requirements.txt and the ymls are in sync |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on Mac. It works fine
Resolves recent issue #2126