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

Consolidate requirements #174

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

forslund
Copy link
Collaborator

@forslund forslund commented Aug 5, 2020

This removes the requirements.txt and package list in setup.py in favor of the list in requirements/prod.txt and should resolve #172

It also bumps the numpy version to 1.16.2 to match the version listed in requirements.txt which should fix some errors with pyinstaller.

Fixes ModuleNotFound issue with pyinstaller
@NoamDev NoamDev mentioned this pull request Sep 21, 2020
@jessecooper
Copy link

Any reason this is hanging around for over a year?

@NoamDev
Copy link

NoamDev commented Sep 29, 2021

Maybe @MatthewScholefield or @krisgesling can review this?

Copy link
Contributor

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the nudge, this looks like a great change.

One question below

@@ -1,44 +0,0 @@
absl-py==0.7.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What were the rest of these requirements for?

Do we need a new requirements/dev.txt or something?

@@ -1,4 +1,4 @@
numpy==1.16
numpy==1.16.2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be bumped to 1.16.5

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also h5py<3.0.0 fixes a runtime problem

@ShamoX
Copy link

ShamoX commented Jul 21, 2022

Hello,

Since I need that project, I decided to fork and advance. Sorry for the inconvenience but it can be reversed if Mycroft wants of course).

You will find your PR to the fork here.

Again, sorry for the inconvenient. I created a thread on Mycroft community forum to discuss if needed.

@ShamoX
Copy link

ShamoX commented Jul 24, 2022

Be careful, those changes generate a regression: wheel cannot build anymore, since setup.py needs requirements/prod.txt to be part of the package and is not.

I revert the change and propose to remove entirely requirements/prod.txt and use correctly setup.py list: https://gitlab.com/liant-sasu/mycroft-precise/-/merge_requests/13

@jessecooper
Copy link

@ShamoX https://github.com/jessecooper/mycroft-precise/tree/dev/requirements
This is what I am currently running with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Medium priority
Development

Successfully merging this pull request may close these issues.

Requirements mismatch as defined in multiple locations
5 participants