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

Create concise requirement and env files #17

Closed
andrewtavis opened this issue Apr 7, 2021 · 7 comments
Closed

Create concise requirement and env files #17

andrewtavis opened this issue Apr 7, 2021 · 7 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed question Further information is requested

Comments

@andrewtavis
Copy link
Owner

This issue is for creating concise versions of requirements.txt and environment.yml for wikirepo. It would be great if these files were created by hand with specific version numbers or generated in a way so that sub-dependencies don't always need to be updated.

As of now both files are being created with the following commands in the package's conda virtual environment:

pip list --format=freeze > requirements.txt  
conda env export --no-builds | grep -v "^prefix: " > environment.yml

wikirepo and other obviously unneeded packages are then removed from these files before being uploaded.

Any insights or help would be much appreciated!

@andrewtavis andrewtavis added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed question Further information is requested and removed enhancement New feature or request labels Apr 7, 2021
@andrewtavis
Copy link
Owner Author

Hi @djKooks, posting this in the issue rather than the PR :) Thanks for your help with this!

For setup.py, would your basic suggestion on this be to pick the package that has the most requirements (pandas in this case that covers scipy, numpy, etc), and then build around it with what it doesn't include?

Also, could you give a quick explanation of why things like pylint, pyOpenSSL, packaging, and certifi are needed in the requirements files? Do you just know to leave these in, or was it a bit of trial and error to see what'd work? Testing packages make sense, but the others I'm not sure on. And why is numpy still in the requirements (as it's covered by pandas in setup.py)?

@kination
Copy link
Contributor

@andrewtavis hello~sorry for late reply...

For setup.py, would your basic suggestion on this be to pick the package that has the most requirements (pandas in this case that covers scipy, numpy, etc), and then build around it with what it doesn't include?

Yes. Also I've change setup.py to get requirements from requirements.txt.

pylint, pyOpenSSL, packaging, and certifi are needed in the requirements files?

In my opinion pylint and packaging can be used for the project(for linting, packaging in the future). If you think this is useless(or willing to use other module such as black or yapf for linting...) you could remove it in the future.

And why is numpy still in the requirements

Seems you're correct, but because there are parts using 'numpy' in your code, will be better to check if it works well without this...(didn't tried yet)

@kination
Copy link
Contributor

Also, I think maybe we could just remove 'environment.yml', cause AFAIK conda can be used with requirements.txt.

@andrewtavis
Copy link
Owner Author

andrewtavis commented Apr 14, 2021

@djKooks, no worries at all - reply wasn't late :)

I accepted the most recent changes to setup.py. Very much appreciate them!

Let's leave pylint and packaging in then, but the codestyle we're using is black, so should that be added? I'm fine with leaving numpy in there as well, as maybe there could be an issue with the documentation if it's not explicitly in there. Things to worry about later :)

I guess let's also just leave environment.yml for folks, as one benefit is that that will give people an environment that has conda packages installed in that manner and the rest with pip - for if a future contributor wants to use conda to manage their version of it. Unsure if conda tries to first conda install and then reverts to pip, but it'll be more explicit with an environment.yml, and it seems that other projects are maintaining both 😊

Thanks again, and by all means let me know if you have further suggestions!

@andrewtavis
Copy link
Owner Author

Let me know on if black should be added, and then I'lll give it a close based on that :)

@kination
Copy link
Contributor

@andrewtavis because linting tools(black, pylint, yapf) are doing similar things, so you could just decide which one to use.

@andrewtavis
Copy link
Owner Author

Ok, great. I'll swap in black and close this then :)

My best for now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants