-
Notifications
You must be signed in to change notification settings - Fork 397
Setuptools #402
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
Setuptools #402
Conversation
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.
It looks like this actually replaces #401, doesn't it? Apart from a couple minor comments, it looks fine to me.
setup.py
Outdated
from distutils.dist import Distribution | ||
from distutils.util import convert_path | ||
from distutils import ccompiler, sysconfig | ||
from setuptools.dist import Distribution |
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.
Duplicated above.
setup.py
Outdated
req | ||
for req in content.split("\n") | ||
if req != '' and not req.startswith('#') | ||
] |
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.
style quibble: I don't like the closing paren being indented that way. I would probably put the read
functionality inside the get_install_requirements
function, since it isn't used anywhere else, and then format the return as
return [req for req in content.split("\n")
if req and not req.startswith('#')]
could you rebase this onto master to make it easier to see the remaining differences? |
This is ready for another round of reviews. |
Is it basically assumed now that people have setuptools installed? I guess it is pretty much safe because matplotlib has been requiring setuptools for awhile now, right? |
Yep. |
# Do not merge this before #400 and #401... and simplify how we deal with the requirements in
setup.py
.