Skip to content

CLN: Standardize indentation and alphabetize deps #18104

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

Merged
merged 1 commit into from
Nov 4, 2017

Conversation

jbrockmendel
Copy link
Member

I can't fix a problem that makes zero sense to me (#18089), but in the interests of being a team player, I can try to contribute around the periphery.

This just takes the ext_data dict and a) standardizes the indentation, b) arranges the entries in alphabetical order. It is straightforward to check that the version of the dict here == the version in master. The main reason to do this is so that the diff in the next step is easy to review.

The next step is to go through and identify pyx files that don't cimport anything e.g. _libs.testing and fill out their ext_data entries with the appropriate []s (note that the processing of ext_data inserts default values for missing keys, so adding [] entries is part of the specification).

There are a few modules that meet that description, then a handful of others that only have cimports from numpy (e.g. io.sas._sas). Those entries can be fully filled out independently of any other outstanding issues.

@gfyoung gfyoung added Clean Build Library building on various platforms labels Nov 4, 2017
@gfyoung gfyoung changed the title standardize indentation, arrange in alphabetical order CLN: Standardize indentation and alphabetize deps Nov 4, 2017
@jreback
Copy link
Contributor

jreback commented Nov 4, 2017

this is fine (assume it lints ok)
ping on green

@jreback jreback added this to the 0.22.0 milestone Nov 4, 2017
@jreback
Copy link
Contributor

jreback commented Nov 4, 2017

as far as the other issue
i would simply add pxd as appropriate

@codecov
Copy link

codecov bot commented Nov 4, 2017

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18104      +/-   ##
==========================================
- Coverage   91.27%   91.26%   -0.02%     
==========================================
  Files         163      163              
  Lines       50123    50123              
==========================================
- Hits        45752    45744       -8     
- Misses       4371     4379       +8
Flag Coverage Δ
#multiple 89.07% <ø> (ø) ⬆️
#single 40.32% <ø> (-0.06%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.75% <0%> (-0.1%) ⬇️
pandas/core/indexes/datetimes.py 95.5% <0%> (+0.09%) ⬆️

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 5f11353...fa71d62. Read the comment docs.

@jreback jreback merged commit 3268f4e into pandas-dev:master Nov 4, 2017
@jbrockmendel
Copy link
Member Author

as far as the other issue
i would simply add pxd as appropriate

You're gonna have to be more specific. Or for the sake of morale, let me ignore this comment and move on with my day.

@jbrockmendel jbrockmendel deleted the specify branch November 4, 2017 18:20
@gfyoung
Copy link
Member

gfyoung commented Nov 4, 2017

@jreback : IMO we should just make sure that we get a deps list that works first and foremost. We can cut it down afterwards. I think that's why @jbrockmendel has been confused by what to do there.

1kastner pushed a commit to 1kastner/pandas that referenced this pull request Nov 5, 2017
No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms Clean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants