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.

Sign up for GitHub

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

Jump to bottom

BLD: since we already use setuptools, let's remove the optional logic… #18448

Closed
wants to merge 2 commits into from
Closed

BLD: since we already use setuptools, let's remove the optional logic… #18448

wants to merge 2 commits into from

Conversation

gkonefal-reef
Copy link
Contributor

… in setup.py (GH18113).

@codecov
Copy link

codecov bot commented Nov 23, 2017

Codecov Report

Merging #18448 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18448      +/-   ##
==========================================
- Coverage   91.35%   91.31%   -0.05%     
==========================================
  Files         163      163              
  Lines       49691    49691              
==========================================
- Hits        45397    45376      -21     
- Misses       4294     4315      +21
Flag Coverage Δ
#multiple 89.11% <ø> (-0.03%) ⬇️
#single 39.67% <ø> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/plotting/_converter.py 63.44% <0%> (-1.82%) ⬇️
pandas/core/frame.py 97.8% <0%> (-0.1%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c634352...62254e5. Read the comment docs.

@codecov
Copy link

codecov bot commented Nov 23, 2017 •
edited
Loading

Codecov Report

Merging #18448 into master will decrease coverage by 0.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18448      +/-   ##
==========================================
- Coverage   91.46%   91.31%   -0.16%     
==========================================
  Files         157      163       +6     
  Lines       51439    49691    -1748     
==========================================
- Hits        47051    45376    -1675     
+ Misses       4388     4315      -73
Flag Coverage Δ
#multiple 89.11% <ø> (-0.21%) ⬇️
#single 39.67% <ø> (-1.04%) ⬇️
Impacted Files Coverage Δ
pandas/core/computation/expressions.py 0% <0%> (-94.96%) ⬇️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/io/msgpack/_version.py 44.65% <0%> (-55.35%) ⬇️
pandas/io/clipboard/clipboards.py 24.05% <0%> (-8.14%) ⬇️
pandas/plotting/_converter.py 63.44% <0%> (-1.82%) ⬇️
pandas/core/indexes/interval.py 92.52% <0%> (-0.53%) ⬇️
pandas/core/base.py 96.56% <0%> (-0.21%) ⬇️
pandas/core/indexes/datetimes.py 95.49% <0%> (-0.21%) ⬇️
pandas/core/generic.py 95.73% <0%> (-0.18%) ⬇️
pandas/core/reshape/merge.py 94.26% <0%> (-0.15%) ⬇️
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e16818...21cbe79. Read the comment docs.

@jbrockmendel jbrockmendel mentioned this pull request Nov 23, 2017
Closed
4 tasks
@jreback jreback added the Build Library building on various platforms label Nov 23, 2017
@jreback jreback added this to the 0.22.0 milestone Nov 23, 2017
jreback
jreback reviewed Nov 23, 2017
View reviewed changes
setup.py Outdated

if _have_setuptools:
setuptools_kwargs["test_suite"] = "nose.collector"
setuptools_kwargs["test_suite"] = "nose.collector"
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this, its no longer needed

jreback
jreback requested changes Nov 23, 2017
View reviewed changes
setup.py Outdated
@@ -56,10 +50,6 @@ def is_platform_mac():
'pytz >= 2011k',
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be a bit simpler, e.g. use a common setuptool_kwargs, leaving out datetutil, then add this separately for PY2 and 3

bashtage
bashtage reviewed Nov 24, 2017
View reviewed changes
doc/source/whatsnew/v0.22.0.txt Outdated
@@ -195,5 +195,6 @@ Other
^^^^^

- Improved error message when attempting to use a Python keyword as an identifier in a numexpr query (:issue:`18221`)
- BLD: since we already use setuptools, let's remove the optional logic in setup.py (:issue:`18113`)
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't the usual style of a what's new entry. Something like "Simplified setup due to explicit dependence on setuptools (:issue...)"

@jreback
Copy link
Contributor

jreback commented Nov 28, 2017

can you rebase and update

@pep8speaks
Copy link

Hello @gkonefal-reef! Thanks for updating the PR.

  • In the file setup.py, following are the PEP8 issues :

Line 47:80: E501 line too long (82 > 79 characters)

@gkonefal-reef
Copy link
Contributor Author

I've applied changes as it was requested.

jreback pushed a commit to jreback/pandas that referenced this pull request Dec 3, 2017
@gkonefal-reef @jreback
BLD: since we already use setuptools, let's remove the optional logic…
c6b88e1 
closes pandas-dev#18113

Author: Grzegorz Konefał <grzegorz.konefal@reef.pl>
Author: Krzysztof Chomski <krzysztof.chomski@reef.pl>

Closes pandas-dev#18448 from gkonefal-reef/GH18113 and squashes the following commits:

21cbe79 [Grzegorz Konefał] Comments applied
290b49c [Krzysztof Chomski] BLD: since we already use setuptools, let's remove the optional logic in setup.py (GH18113).
@jreback
Copy link
Contributor

jreback commented Dec 3, 2017

ok re-running on travis here just to make sure

jreback pushed a commit to jreback/pandas that referenced this pull request Dec 3, 2017
@gkonefal-reef @jreback
BLD: since we already use setuptools, let's remove the optional logic…
b55b816 
closes pandas-dev#18113

Author: Grzegorz Konefał <grzegorz.konefal@reef.pl>
Author: Krzysztof Chomski <krzysztof.chomski@reef.pl>

Closes pandas-dev#18448 from gkonefal-reef/GH18113 and squashes the following commits:

21cbe79 [Grzegorz Konefał] Comments applied
290b49c [Krzysztof Chomski] BLD: since we already use setuptools, let's remove the optional logic in setup.py (GH18113).
@jreback jreback closed this in a9e4731 Dec 3, 2017
@jreback
Copy link
Contributor

jreback commented Dec 3, 2017

thanks @gkonefal-reef

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bashtage bashtage bashtage left review comments

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Build Library building on various platforms
Projects
None yet
Milestone
0.23.0
Development

Successfully merging this pull request may close these issues.

BLD: since we already use setuptools, let's remove the optional logic in setup.py
4 participants
@gkonefal-reef @jreback @pep8speaks @bashtage