Skip to content

DOC: Fixed some pep8 violations, arr.size instead of len(arr)#4219

Merged
josef-pkt merged 1 commit intostatsmodels:masterfrom
tommyod:kde_review
Jan 22, 2018
Merged

DOC: Fixed some pep8 violations, arr.size instead of len(arr)#4219
josef-pkt merged 1 commit intostatsmodels:masterfrom
tommyod:kde_review

Conversation

@tommyod
Copy link
Contributor

@tommyod tommyod commented Jan 21, 2018

I read through some files in statsmodels/nonparametric and found some pep8 violations which I fixed. Very small stufff, mostly spaces. Remoted a few unused imports. Made one change to the code: using arr.size is faster than len(arr).

Comments very welcome.

@coveralls
Copy link

coveralls commented Jan 21, 2018

Coverage Status

Coverage increased (+0.0006%) to 82.171% when pulling 69b05ef on tommyod:kde_review into 0d757e5 on statsmodels:master.

@codecov-io
Copy link

codecov-io commented Jan 21, 2018

Codecov Report

Merging #4219 into master will increase coverage by <.01%.
The diff coverage is 84.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4219      +/-   ##
==========================================
+ Coverage   79.56%   79.56%   +<.01%     
==========================================
  Files         562      562              
  Lines       83843    83840       -3     
  Branches     9553     9553              
==========================================
- Hits        66711    66709       -2     
+ Misses      14948    14946       -2     
- Partials     2184     2185       +1
Impacted Files Coverage Δ
statsmodels/nonparametric/bandwidths.py 92.85% <ø> (ø) ⬆️
statsmodels/nonparametric/kdetools.py 70.96% <83.33%> (ø) ⬆️
statsmodels/nonparametric/kde.py 85.81% <85.71%> (+0.39%) ⬆️
statsmodels/stats/descriptivestats.py 24.13% <0%> (ø) ⬆️

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 0d757e5...69b05ef. Read the comment docs.

@josef-pkt
Copy link
Member

Made one change to the code: using arr.size is faster than len(arr).

Is there much of a speed difference? How does shape[0] compare?
The problem is that size has a different meaning than len or shape[0]. Even if it's not currently used, I prefer code that would allow extensions. (Less to think about when code is changed because our standard is to have observations in rows.)

And thanks for pointing out the non-style changes in the description. That makes it much easier to quickly check the changes.

@tommyod
Copy link
Contributor Author

tommyod commented Jan 22, 2018

@josef-pkt Upon further investigation, the speed difference is practically insignificant. I have removed this code change from the PR. The unused imports and the pep8 improvements are still present.

@josef-pkt
Copy link
Member

@tommyod Thanks
Can you squash this into one commit, then it looks ready to merge.

pure pep-8 style changes, commit prefix STY
we roughly follow this
http://www.statsmodels.org/stable/dev/maintainer_notes.html?highlight=prefix#commit-comments

@josef-pkt josef-pkt merged commit c99558b into statsmodels:master Jan 22, 2018
@josef-pkt
Copy link
Member

merged, Thanks @tommyod

@josef-pkt josef-pkt added this to the 0.9 milestone Apr 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants