Skip to content

Fix 18121: Add .pxd linting to lint.sh #18150

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 6 commits into from
Nov 9, 2017

Conversation

manrajgrover
Copy link
Contributor

ci/lint.sh Outdated
@@ -42,6 +42,18 @@ if [ "$LINT" ]; then
done
echo "Linting *.pxi.in DONE"

echo "Linting *.pxd"
for path in 'src'
Copy link
Contributor

Choose a reason for hiding this comment

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

this should recursively search _libs

Copy link
Contributor

Choose a reason for hiding this comment

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

i think there may be some fixes needed as well to make this pass (to the actual .pxd files)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback Got it. Is there a way to make lint.sh run locally?

Copy link
Contributor

Choose a reason for hiding this comment

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

not really but you can just check the command locally

Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to make lint.sh run locally?

On OSX/Linux:

export LINT=TRUE; /bin/bash ci/lint.sh

The first line of output is an error message you can ignore.

@gfyoung gfyoung added Clean Internals Related to non-user accessible pandas implementation Code Style Code style, linting, code_checks labels Nov 7, 2017
@jreback
Copy link
Contributor

jreback commented Nov 8, 2017

rebase on master

@manrajgrover manrajgrover force-pushed the fix-18121-add-pxd-lint branch from 0eeb816 to d6a7e5d Compare November 8, 2017 11:03
#object PyArray_NewFromDescr (type, dtype, int, npy_intp *, npy_intp *, void *, int, object)
object PyArray_New (type, int, npy_intp *, int, npy_intp *,
void *, int, int, object)
#object PyArray_NewFromDescr \
Copy link
Contributor

Choose a reason for hiding this comment

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

you can't use \ here, this is kind of like a c-header file. you can just remove this line as its not used anyhow (its commented out)

@codecov
Copy link

codecov bot commented Nov 9, 2017

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18150      +/-   ##
==========================================
- Coverage    91.4%   91.38%   -0.02%     
==========================================
  Files         163      163              
  Lines       50073    50068       -5     
==========================================
- Hits        45769    45755      -14     
- Misses       4304     4313       +9
Flag Coverage Δ
#multiple 89.19% <ø> (-0.01%) ⬇️
#single 40.36% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.8% <0%> (-0.1%) ⬇️
pandas/core/indexes/datetimes.py 95.48% <0%> (-0.04%) ⬇️
pandas/core/indexes/timedeltas.py 91.17% <0%> (-0.02%) ⬇️
pandas/io/parquet.py 65.38% <0%> (ø) ⬆️
pandas/core/groupby.py 92.04% <0%> (ø) ⬆️
pandas/compat/__init__.py 58.1% <0%> (ø) ⬆️
pandas/core/indexes/datetimelike.py 97.11% <0%> (ø) ⬆️
pandas/core/indexes/period.py 92.89% <0%> (+0.01%) ⬆️
pandas/core/tools/timedeltas.py 98.41% <0%> (+0.02%) ⬆️

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 93c755e...4b7caa6. Read the comment docs.

@codecov
Copy link

codecov bot commented Nov 9, 2017

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18150      +/-   ##
==========================================
- Coverage    91.4%   91.38%   -0.02%     
==========================================
  Files         163      163              
  Lines       50073    50068       -5     
==========================================
- Hits        45769    45755      -14     
- Misses       4304     4313       +9
Flag Coverage Δ
#multiple 89.19% <ø> (-0.01%) ⬇️
#single 40.36% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.8% <0%> (-0.1%) ⬇️
pandas/core/indexes/datetimes.py 95.48% <0%> (-0.04%) ⬇️
pandas/core/indexes/timedeltas.py 91.17% <0%> (-0.02%) ⬇️
pandas/core/groupby.py 92.04% <0%> (ø) ⬆️
pandas/io/parquet.py 65.38% <0%> (ø) ⬆️
pandas/compat/__init__.py 58.1% <0%> (ø) ⬆️
pandas/core/indexes/datetimelike.py 97.11% <0%> (ø) ⬆️
pandas/core/indexes/period.py 92.89% <0%> (+0.01%) ⬆️
pandas/core/tools/timedeltas.py 98.41% <0%> (+0.02%) ⬆️

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 93c755e...4b7caa6. Read the comment docs.

@jreback jreback added this to the 0.22.0 milestone Nov 9, 2017
@jreback jreback merged commit 17e0b13 into pandas-dev:master Nov 9, 2017
@jreback
Copy link
Contributor

jreback commented Nov 9, 2017

thanks @manrajgrover

watercrossing pushed a commit to watercrossing/pandas that referenced this pull request Nov 10, 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
Clean Code Style Code style, linting, code_checks Internals Related to non-user accessible pandas implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: add .pxd linting to lint.sh
4 participants