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

CLN: Use pandas.core.common for None checks #17816

Merged
merged 2 commits into from
Oct 13, 2017

Conversation

jschendel
Copy link
Member

  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

Used None checking functions from pandas.core.common where applicable. Common patterns changed:

  • all(v is not None for v in values) -> _all_not_none(*values)
    • Used * for argument unpacking in many cases, not sure if that's frowned upon.
  • a is not None and b is not None and c is not None -> _all_not_none(a, b, c)
    • Only did this for 3+ checks against None. Left checks of 2 as-is.

Also created an any_not_none function similar to the other three that exist. I suppose the is equivalent to not all_none(...), but I feel like using any_not_none is more readable and indicative of intent.

@codecov
Copy link

codecov bot commented Oct 8, 2017

Codecov Report

Merging #17816 into master will decrease coverage by <.01%.
The diff coverage is 94.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17816      +/-   ##
==========================================
- Coverage   91.26%   91.25%   -0.01%     
==========================================
  Files         163      163              
  Lines       49978    49981       +3     
==========================================
- Hits        45611    45609       -2     
- Misses       4367     4372       +5
Flag Coverage Δ
#multiple 89.05% <94.64%> (ø) ⬆️
#single 40.26% <50%> (-0.05%) ⬇️
Impacted Files Coverage Δ
pandas/util/testing.py 100% <ø> (ø) ⬆️
pandas/core/indexes/api.py 98.78% <ø> (ø) ⬆️
pandas/io/formats/style.py 100% <ø> (ø) ⬆️
pandas/core/reshape/merge.py 94.26% <100%> (ø) ⬆️
pandas/core/common.py 92.81% <100%> (+1.38%) ⬆️
pandas/core/indexes/range.py 92.85% <100%> (+0.02%) ⬆️
pandas/core/panel.py 96.94% <100%> (-0.01%) ⬇️
pandas/core/groupby.py 92.03% <100%> (-0.02%) ⬇️
pandas/io/json/table_schema.py 95.83% <100%> (+0.05%) ⬆️
pandas/core/series.py 94.89% <100%> (ø) ⬆️
... and 11 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 7db7f82...2572d95. Read the comment docs.

@codecov
Copy link

codecov bot commented Oct 8, 2017

Codecov Report

Merging #17816 into master will decrease coverage by <.01%.
The diff coverage is 94.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17816      +/-   ##
==========================================
- Coverage   91.26%   91.25%   -0.01%     
==========================================
  Files         163      163              
  Lines       49978    49981       +3     
==========================================
- Hits        45611    45609       -2     
- Misses       4367     4372       +5
Flag Coverage Δ
#multiple 89.05% <94.64%> (ø) ⬆️
#single 40.26% <50%> (-0.05%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/api.py 98.78% <ø> (ø) ⬆️
pandas/io/formats/style.py 100% <ø> (ø) ⬆️
pandas/util/testing.py 100% <ø> (ø) ⬆️
pandas/io/formats/excel.py 96.71% <100%> (+0.01%) ⬆️
pandas/io/json/table_schema.py 95.83% <100%> (+0.05%) ⬆️
pandas/core/indexes/range.py 92.85% <100%> (+0.02%) ⬆️
pandas/core/panel.py 96.94% <100%> (-0.01%) ⬇️
pandas/core/series.py 94.89% <100%> (ø) ⬆️
pandas/core/reshape/merge.py 94.26% <100%> (ø) ⬆️
pandas/core/indexes/multi.py 96.39% <100%> (ø) ⬆️
... and 11 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 7db7f82...2572d95. Read the comment docs.

@codecov
Copy link

codecov bot commented Oct 8, 2017

Codecov Report

Merging #17816 into master will decrease coverage by 0.03%.
The diff coverage is 94.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17816      +/-   ##
==========================================
- Coverage   91.25%   91.21%   -0.04%     
==========================================
  Files         163      163              
  Lines       50038    50041       +3     
==========================================
- Hits        45661    45647      -14     
- Misses       4377     4394      +17
Flag Coverage Δ
#multiple 89.02% <94.64%> (-0.02%) ⬇️
#single 40.28% <50%> (-0.05%) ⬇️
Impacted Files Coverage Δ
pandas/util/testing.py 100% <ø> (ø) ⬆️
pandas/core/indexes/api.py 98.78% <ø> (ø) ⬆️
pandas/io/formats/style.py 100% <ø> (ø) ⬆️
pandas/io/json/table_schema.py 95.83% <100%> (+0.05%) ⬆️
pandas/core/indexes/range.py 92.85% <100%> (+0.02%) ⬆️
pandas/core/indexes/base.py 96.47% <100%> (-0.01%) ⬇️
pandas/core/reshape/concat.py 97.6% <100%> (ø) ⬆️
pandas/io/formats/format.py 95.94% <100%> (ø) ⬆️
pandas/core/groupby.py 91.98% <100%> (-0.02%) ⬇️
pandas/core/generic.py 92.2% <100%> (ø) ⬆️
... and 12 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 7e159ae...9599722. Read the comment docs.

for arg in args:
if arg is not None:
return False
return True
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a 1-liner doc-string to these, otherwise lgtm.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done and green.

@jreback jreback merged commit 3c964a4 into pandas-dev:master Oct 13, 2017
@jreback
Copy link
Contributor

jreback commented Oct 13, 2017

thanks! this helps make things consistent. Ideally I think we could expand the dev docs to give a little color on where things are (e.g. we have lots of utilities, but not always easily discovered). if you would want to do some docs would be fantastic.

buntwo pushed a commit to buntwo/pandas that referenced this pull request Oct 14, 2017
ghost pushed a commit to reef-technologies/pandas that referenced this pull request Oct 16, 2017
ghost pushed a commit to reef-technologies/pandas that referenced this pull request Oct 16, 2017
* upstream/master: (76 commits)
  CategoricalDtype construction: actually use fastpath (pandas-dev#17891)
  DEPR: Deprecate tupleize_cols in to_csv (pandas-dev#17877)
  BUG: Fix wrong column selection in drop_duplicates when duplicate column names (pandas-dev#17879)
  DOC: Adding examples to update docstring (pandas-dev#16812) (pandas-dev#17859)
  TST: Skip if no openpyxl in test_excel (pandas-dev#17883)
  TST: Catch read_html slow test warning (pandas-dev#17874)
  flake8 cleanup (pandas-dev#17873)
  TST: remove moar warnings (pandas-dev#17872)
  ENH: tolerance now takes list-like argument for reindex and get_indexer. (pandas-dev#17367)
  ERR: Raise ValueError when week is passed in to_datetime format witho… (pandas-dev#17819)
  TST: remove some deprecation warnings (pandas-dev#17870)
  Refactor index-as-string groupby tests and fix spurious warning (Bug 17383) (pandas-dev#17843)
  BUG: merging with a boolean/int categorical column (pandas-dev#17841)
  DEPR: Deprecate read_csv arguments fully (pandas-dev#17865)
  BUG: to_json - prevent various segfault conditions (GH14256) (pandas-dev#17857)
  CLN: Use pandas.core.common for None checks (pandas-dev#17816)
  BUG: set tz on DTI from fixed format HDFStore (pandas-dev#17844)
  RLS: v0.21.0rc1
  Whatsnew cleanup (pandas-dev#17858)
  DEPR: Deprecate the convert parameter completely (pandas-dev#17831)
  ...
@jschendel jschendel deleted the cln-check-none branch October 19, 2017 23:16
alanbato pushed a commit to alanbato/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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants