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

ARROW-2325: [Python] Update setup.py to use Markdown project description #1811

Closed

Conversation

@AlexHagerman
Copy link
Contributor

commented Mar 30, 2018

Any thoughts on the best way to make sure the pip upload uses twine>=1.11.0 and setuptools>=38.6.0? I don't see a reference to the twine version in https://github.com/apache/arrow/blob/master/dev/release/RELEASE_MANAGEMENT.md but maybe adding a note? Or maybe it should be documented and part of the packaging discussion?

@AlexHagerman AlexHagerman changed the title Arrow-2325: Update setup.py to use Markdown project description Arrow-2325: [Python] Update setup.py to use Markdown project description Mar 30, 2018

@wesm wesm changed the title Arrow-2325: [Python] Update setup.py to use Markdown project description ARROW-2325: [Python] Update setup.py to use Markdown project description Apr 1, 2018

@wesm

wesm approved these changes Apr 1, 2018

Copy link
Member

left a comment

+1. Feel free to document in RELEASE_MANAGEMENT.md

@@ -494,6 +493,7 @@ def parse_version(root):
tests_require=['pytest', 'pandas'],
description="Python library for Apache Arrow",
long_description=long_description,
long_description_content_type="text/markdown",

This comment has been minimized.

Copy link
@pitrou

pitrou Apr 2, 2018

Contributor

Are unknown arguments simply ignored? I'm thinking of people who may want to build Arrow using an older setuptools.

This comment has been minimized.

Copy link
@AlexHagerman

AlexHagerman Apr 2, 2018

Author Contributor

I can create a new environment with setuptools < 38.6.0 to see if building breaks.

This comment has been minimized.

Copy link
@AlexHagerman

AlexHagerman Apr 2, 2018

Author Contributor

I get this warning: UserWarning: Unknown distribution options: 'long_description_content_type' warnings.warn(msg) but the build still completes when running python setup.py build_ext --build-type=$ARROW_BUILD_TYPE --with-parquet --with-plasma --inplace do you have any suggestions on a more formal or exhaustive way to check on this?

This comment has been minimized.

Copy link
@pitrou

pitrou Apr 2, 2018

Contributor

No, it's fine with a warning, as long as the build succeeds.

@@ -39,6 +39,8 @@
from distutils.util import strtobool
from distutils import sysconfig

from setuptools import setup

This comment has been minimized.

Copy link
@pitrou

pitrou Apr 2, 2018

Contributor

That import already exists above.

This comment has been minimized.

Copy link
@AlexHagerman

AlexHagerman Apr 2, 2018

Author Contributor

Not sure why I missed the existing and added this import. I'll get that corrected and also setup my linter to find duplicate imports. Thanks for catching.

@AlexHagerman

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2018

@pitrou I realize I've been using the flake8 commands from the development doc flake8 pyarrow
and flake8 --config=.flake8.cython pyarrow. When I run flake8 at the python folder level I see a few different lint errors, and the reimport of setuptools in setup.py prior to the removal. I will run flake8 at the python folder level in the future, but would it be worth opening a ticket or two about cleaning up the linter errors and then having the build run on the python folder not just pyarrow?
flake8_arrow

@xhochy

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

@AlexHagerman We should also run flake8 on these files but we can make that adjustment in a separate PR. We sadly cannot run it on the whole arrow/python directory as there will typically be other files like a developer's virtualenv.

@xhochy

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

Any thoughts on the best way to make sure the pip upload uses twine>=1.11.0 and setuptools>=38.6.0? I don't see a reference to the twine version in https://github.com/apache/arrow/blob/master/dev/release/RELEASE_MANAGEMENT.md but maybe adding a note? Or maybe it should be documented and part of the packaging discussion?

Adding a note to https://github.com/apache/arrow/blob/master/dev/release/RELEASE_MANAGEMENT.md should be enough for now. We need build better infrastructure for our release but this is sadly more mid-term than short-term.

@AlexHagerman

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2018

@xhochy I went through the appveyor and travis files to get a better understanding before updating the release documentation. In arrow-dist/scripts/https://github.com/apache/arrow-dist/blob/master/scripts/python-wheels-windows.bat I see a new conda environment being created and setuptools is defaulting to 39.0.1 for new environments today, so we are good there. I was wondering if you could point me towards understanding when/how an environment is created with .travis.yml? Wanted to make sure setuptools would be up to date, or open a ticket if we needed to add something like SETUPTOOLS_DEP>="38.6.0".

@xhochy

This comment has been minimized.

Copy link
Member

commented Apr 5, 2018

@AlexHagerman We only use conda and virtualenv to create environments in Travis and AppVeyor. Both install the latest version of setuptools on the creation of new environments (conda is more explicit about that). So we're fine with both of them in the CI.

@xhochy

xhochy approved these changes Apr 5, 2018

Copy link
Member

left a comment

+1, LGTM

@xhochy xhochy closed this in 02b0c72 Apr 5, 2018

@AlexHagerman AlexHagerman deleted the AlexHagerman:setuppy_longdescription branch May 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.