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: replace %s syntax with .format in pandas.io.parsers #24721

Merged
merged 1 commit into from
Mar 19, 2019

Conversation

makeajourney
Copy link
Contributor

progress towards #16130

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

@gfyoung gfyoung added Strings String extension data type and string data Clean labels Jan 11, 2019
@makeajourney
Copy link
Contributor Author

The error occurred on python 2.7 from CI.
Is there any person who knows how I can run the test on python 2.7? I wanna know the environment setting.

@WillAyd
Copy link
Member

WillAyd commented Jan 12, 2019

@makeajourney you could create a conda environment using Python 2.7 locally to test

@WillAyd
Copy link
Member

WillAyd commented Jan 31, 2019

@makeajourney can you merge master here?

@makeajourney makeajourney force-pushed the issue-16130-io-parsers branch from e81c43b to c64ae19 Compare January 31, 2019 14:10
@codecov
Copy link

codecov bot commented Jan 31, 2019

Codecov Report

Merging #24721 into master will increase coverage by <.01%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24721      +/-   ##
==========================================
+ Coverage   92.37%   92.38%   +<.01%     
==========================================
  Files         166      166              
  Lines       52397    52321      -76     
==========================================
- Hits        48404    48338      -66     
+ Misses       3993     3983      -10
Flag Coverage Δ
#multiple 90.8% <94.11%> (ø) ⬆️
#single 43.06% <5.88%> (+0.18%) ⬆️
Impacted Files Coverage Δ
pandas/io/parsers.py 95.4% <94.11%> (+0.05%) ⬆️
pandas/tseries/frequencies.py 97.08% <0%> (-0.63%) ⬇️
pandas/core/arrays/timedeltas.py 87.86% <0%> (-0.45%) ⬇️
pandas/core/reshape/merge.py 94.26% <0%> (-0.22%) ⬇️
pandas/plotting/_timeseries.py 65.28% <0%> (-0.18%) ⬇️
pandas/core/computation/expr.py 88.57% <0%> (-0.12%) ⬇️
pandas/core/reshape/tile.py 94.88% <0%> (-0.06%) ⬇️
pandas/core/arrays/categorical.py 95.93% <0%> (-0.05%) ⬇️
pandas/core/arrays/datetimes.py 97.75% <0%> (-0.05%) ⬇️
pandas/core/arrays/interval.py 93.08% <0%> (-0.04%) ⬇️
... and 39 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 149138e...c64ae19. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 31, 2019

Codecov Report

Merging #24721 into master will increase coverage by <.01%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24721      +/-   ##
==========================================
+ Coverage   91.73%   91.73%   +<.01%     
==========================================
  Files         173      173              
  Lines       52845    52845              
==========================================
+ Hits        48479    48480       +1     
+ Misses       4366     4365       -1
Flag Coverage Δ
#multiple 90.3% <94.11%> (ø) ⬆️
#single 41.72% <5.88%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/io/parsers.py 95.34% <94.11%> (ø) ⬆️
pandas/util/testing.py 87.66% <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 b673188...e8b2a47. Read the comment docs.

@WillAyd WillAyd added this to the 0.25.0 milestone Jan 31, 2019
@jreback
Copy link
Contributor

jreback commented Feb 2, 2019

can you merge master

@makeajourney makeajourney force-pushed the issue-16130-io-parsers branch from c64ae19 to ddb5f74 Compare February 2, 2019 23:10
@@ -2273,10 +2278,11 @@ def __init__(self, f, **kwds):
raise ValueError('Only length-1 decimal markers supported')

if self.thousands is None:
self.nonnum = re.compile('[^-^0-9^%s]+' % self.decimal)
self.nonnum = re.compile(
r'[^-^0-9^{decimal}]+'.format(decimal=self.decimal))
Copy link
Member

Choose a reason for hiding this comment

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

Hmm does converting this to a raw string when it wasn't previously affect the behavior at all? Might also be the root cause of your issue, though we technically drop Py2 support with v0.25 anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Temporarily, I want to try to put that code back for clearing the problem from ci.
And then I'll find out how can I solve the problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change has no problem :)

@jreback jreback removed this from the 0.25.0 milestone Feb 6, 2019
@pep8speaks
Copy link

pep8speaks commented Feb 9, 2019

Hello @makeajourney! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-03-19 19:08:27 UTC

@WillAyd
Copy link
Member

WillAyd commented Feb 19, 2019

Can you merge master and check CI failures?

@makeajourney
Copy link
Contributor Author

I tried to install the python 2.7 using the pyenv, but I failed. At the same time, I tried to install and used the anaconda2, but It was a bad idea. Anyway, I've been finding another way to test with the python2.7 environment. Maybe I can use the docker to test.

@WillAyd
Copy link
Member

WillAyd commented Feb 20, 2019

@makeajourney if you already have conda installed you shouldn't need a separate installation to get a Py27 environment up.

https://docs.conda.io/projects/conda/en/latest/user-guide/tasks/manage-environments.html#creating-an-environment-with-commands

conda create -n pandas-dev2 python=2.7

@makeajourney makeajourney force-pushed the issue-16130-io-parsers branch from 4dc8546 to ec1a300 Compare February 20, 2019 23:02
@@ -2557,7 +2563,8 @@ def _infer_columns(self):

while cur_count > 0:
counts[col] = cur_count + 1
col = "%s.%d" % (col, cur_count)
col = '{column}.{count}'.format(
column=u''.join(col), count=cur_count)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there is a problem with ASCII encoding by python 2.7 on Windows. I've tried to solve this problem by changing some codes. But it does not work.

@WillAyd WillAyd added this to the 0.25.0 milestone Feb 22, 2019
@WillAyd
Copy link
Member

WillAyd commented Feb 22, 2019

OK no worries. We are going to drop Py27 supported with the 0.25 release anyway so I would think it's OK to ignore any Py27 failures for that release (unless @jreback objects).

@makeajourney
Copy link
Contributor Author

Okay.
Then I'm going to drop some commits related to the above problems. That's not good to look.

@makeajourney makeajourney force-pushed the issue-16130-io-parsers branch from a80712c to e8b2a47 Compare February 22, 2019 16:22
@codecov
Copy link

codecov bot commented Feb 22, 2019

Codecov Report

Merging #24721 into master will not change coverage.
The diff coverage is 94.11%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #24721   +/-   ##
=======================================
  Coverage   91.26%   91.26%           
=======================================
  Files         172      172           
  Lines       52965    52965           
=======================================
  Hits        48337    48337           
  Misses       4628     4628
Flag Coverage Δ
#multiple 89.82% <94.11%> (ø) ⬆️
#single 41.74% <5.88%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/io/parsers.py 95.34% <94.11%> (ø) ⬆️

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 c975fc4...59d20d8. Read the comment docs.

@WillAyd
Copy link
Member

WillAyd commented Mar 19, 2019

@makeajourney we just dropped the Py27 CI jobs - want to merge master to give this another shot?

@makeajourney makeajourney force-pushed the issue-16130-io-parsers branch from e8b2a47 to 2f67fea Compare March 19, 2019 10:33
@makeajourney
Copy link
Contributor Author

@WillAyd Thanks for letting me know :) I wish there's no more error 😁

@jreback
Copy link
Contributor

jreback commented Mar 19, 2019

pls merge master

@makeajourney makeajourney force-pushed the issue-16130-io-parsers branch from 2f67fea to 59d20d8 Compare March 19, 2019 19:08
@jreback jreback merged commit 8e54e55 into pandas-dev:master Mar 19, 2019
@jreback
Copy link
Contributor

jreback commented Mar 19, 2019

thanks !

@makeajourney makeajourney deleted the issue-16130-io-parsers branch March 19, 2019 22:12
sighingnow added a commit to sighingnow/pandas that referenced this pull request Mar 20, 2019
* origin/master:
  DOC: clean bug fix section in whatsnew (pandas-dev#25792)
  DOC: Fixed PeriodArray api ref (pandas-dev#25526)
  Move locale code out of tm, into _config (pandas-dev#25757)
  Unpin pycodestyle (pandas-dev#25789)
  Add test for rdivmod on EA array (GH23287) (pandas-dev#24047)
  ENH: Support datetime.timezone objects (pandas-dev#25065)
  Cython language level 3 (pandas-dev#24538)
  API: concat on sparse values (pandas-dev#25719)
  TST: assert_produces_warning works with filterwarnings (pandas-dev#25721)
  make core.config self-contained (pandas-dev#25613)
  CLN: replace %s syntax with .format in pandas.io.parsers (pandas-dev#24721)
  TST: Check pytables<3.5.1 when skipping (pandas-dev#25773)
  DOC: Fix typo in docstring of DataFrame.memory_usage  (pandas-dev#25770)
  Replace dicts with OrderedDicts in groupby aggregation functions (pandas-dev#25693)
  TST: Fixturize tests/frame/test_missing.py (pandas-dev#25640)
  DOC: Improve the docsting of Series.iteritems (pandas-dev#24879)
  DOC: Fix function name. (pandas-dev#25751)
  Implementing iso_week_year support for to_datetime (pandas-dev#25541)
  DOC: clarify corr behaviour when using a callable (pandas-dev#25732)
  remove unnecessary check_output (pandas-dev#25755)

# Conflicts:
#	doc/source/whatsnew/v0.25.0.rst
thoo added a commit to thoo/pandas that referenced this pull request Mar 20, 2019
* upstream/master: (55 commits)
  PERF: Improve performance of StataReader (pandas-dev#25780)
  Speed up tokenizing of a row in csv and xstrtod parsing (pandas-dev#25784)
  BUG: Fix _binop for operators for serials which has more than one returns (divmod/rdivmod). (pandas-dev#25588)
  BUG-24971 copying blocks also considers ndim (pandas-dev#25521)
  CLN: Panel reference from documentation (pandas-dev#25649)
  ENH: Quoting column names containing spaces with backticks to use them in query and eval. (pandas-dev#24955)
  BUG: reading windows utf8 filenames in py3.6 (pandas-dev#25769)
  DOC: clean bug fix section in whatsnew (pandas-dev#25792)
  DOC: Fixed PeriodArray api ref (pandas-dev#25526)
  Move locale code out of tm, into _config (pandas-dev#25757)
  Unpin pycodestyle (pandas-dev#25789)
  Add test for rdivmod on EA array (GH23287) (pandas-dev#24047)
  ENH: Support datetime.timezone objects (pandas-dev#25065)
  Cython language level 3 (pandas-dev#24538)
  API: concat on sparse values (pandas-dev#25719)
  TST: assert_produces_warning works with filterwarnings (pandas-dev#25721)
  make core.config self-contained (pandas-dev#25613)
  CLN: replace %s syntax with .format in pandas.io.parsers (pandas-dev#24721)
  TST: Check pytables<3.5.1 when skipping (pandas-dev#25773)
  DOC: Fix typo in docstring of DataFrame.memory_usage  (pandas-dev#25770)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Strings String extension data type and string data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants