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

Fix order parameters and add decimal to to_string #23614

Merged
merged 14 commits into from
Nov 15, 2018

Conversation

thoo
Copy link
Contributor

@thoo thoo commented Nov 10, 2018

@pep8speaks
Copy link

pep8speaks commented Nov 10, 2018

Hello @thoo! Thanks for updating the PR.

Comment last updated on November 11, 2018 at 23:37 Hours UTC

@codecov
Copy link

codecov bot commented Nov 10, 2018

Codecov Report

Merging #23614 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #23614      +/-   ##
==========================================
- Coverage   92.25%   92.25%   -0.01%     
==========================================
  Files         161      161              
  Lines       51384    51383       -1     
==========================================
- Hits        47405    47404       -1     
  Misses       3979     3979
Flag Coverage Δ
#multiple 90.64% <100%> (-0.01%) ⬇️
#single 42.31% <100%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/io/formats/format.py 97.87% <ø> (-0.01%) ⬇️
pandas/core/frame.py 97.02% <100%> (ø) ⬆️

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 8af7637...ddf42c7. Read the comment docs.

@datapythonista
Copy link
Member

Looks quite nice.

I think it good be good to have one test to see that decimal is working in to_string, and then we need to add to the whatsnew two entries, one for the addition of decimal, and another one for the argument resorting.

@datapythonista datapythonista added Docs IO HTML read_html, to_html, Styler.apply, Styler.applymap Clean labels Nov 10, 2018
@thoo
Copy link
Contributor Author

thoo commented Nov 11, 2018

@datapythonista Please let me know the two entries are not in the right place in whatnews file.

@@ -945,6 +946,7 @@ Other API Changes
- :class:`DateOffset` attribute `_cacheable` and method `_should_cache` have been removed (:issue:`23118`)
- Comparing :class:`Timedelta` to be less or greater than unknown types now raises a ``TypeError`` instead of returning ``False`` (:issue:`20829`)
- :meth:`Index.hasnans` and :meth:`Series.hasnans` now always return a python boolean. Previously, a python or a numpy boolean could be returned, depending on circumstances (:issue:`23294`).
- The order of the arguments of `DataFrame.to_html` and `DataFrame.to_string` is rearranged. (:issue:`23614`)
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 use a :func: ref here

Copy link
Contributor

Choose a reason for hiding this comment

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

say to be consistent with each other

@@ -1451,6 +1451,11 @@ def test_to_string_format_na(self):
'4 4.0 bar')
assert result == expected

def test_to_string_decimal(self):
df = DataFrame({'A': [6.0, 3.1, 2.2]})
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 the issue number

Copy link
Member

Choose a reason for hiding this comment

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

Small thing, but I think we consistently use the format GH 23614.

@gfyoung I think you changed some in a recent PR, can you confirm?

@jreback jreback added this to the 0.24.0 milestone Nov 11, 2018
* upstream/master:
  BUG: Casting tz-aware DatetimeIndex to object-dtype ndarray/Index (pandas-dev#23524)
  BUG: Delegate more of Excel parsing to CSV (pandas-dev#23544)
  API: DataFrame.__getitem__ returns Series for sparse column (pandas-dev#23561)
  CLN: use float64_t consistently instead of double, double_t (pandas-dev#23583)
  DOC: Fix Order of parameters in docstrings (pandas-dev#23611)
  TST: Unskip some Categorical Tests (pandas-dev#23613)
  TST: Fix integer ops comparison test (pandas-dev#23619)
  DOC: Fixes to docstring to add validation to CI (pandas-dev#23560)
  DOC: Remove incorrect periods at the end of parameter types (pandas-dev#23600)
  MAINT: tm.assert_raises_regex --> pytest.raises (pandas-dev#23592)
  DOC: Updating Series.resample and DataFrame.resample docstrings (pandas-dev#23197)
* upstream/master:
  BUILD: Simplifying contributor dependencies (pandas-dev#23522)
  BUG/REF: TimedeltaIndex.__new__ (pandas-dev#23539)
@thoo
Copy link
Contributor Author

thoo commented Nov 12, 2018

@jreback All checks are passed. Could you re-review this ? Thanks.

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

looks good, just two minor things

@@ -20,7 +20,8 @@ New features
the user to override the engine's default behavior to include or omit the
dataframe's indexes from the resulting Parquet file. (:issue:`20768`)
- :meth:`DataFrame.corr` and :meth:`Series.corr` now accept a callable for generic calculation methods of correlation, e.g. histogram intersection (:issue:`22684`)

- :func:`DataFrame.to_string` now accepts ``decimal`` as an argument, allowing
the user to use a decimal separator. (:issue:`23614`)
Copy link
Member

Choose a reason for hiding this comment

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

more than to use as decimal separator, if I understood correctly, it'll allow the user to specify which decimal separator should be used in the output. Isn't it?

@@ -1451,6 +1451,11 @@ def test_to_string_format_na(self):
'4 4.0 bar')
assert result == expected

def test_to_string_decimal(self):
df = DataFrame({'A': [6.0, 3.1, 2.2]})
Copy link
Member

Choose a reason for hiding this comment

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

Small thing, but I think we consistently use the format GH 23614.

@gfyoung I think you changed some in a recent PR, can you confirm?

@jreback
Copy link
Contributor

jreback commented Nov 12, 2018

lgtm. @datapythonista merge when you are satisfied.

* upstream/master:
  BUG: Don't over-optimize memory with jagged CSV (pandas-dev#23527)
  DEPR: Deprecate usecols as int in read_excel (pandas-dev#23635)
  More helpful Stata string length error. (pandas-dev#23629)
  BUG: astype fill_value for SparseArray.astype (pandas-dev#23547)
  CLN: datetimelike arrays: isort, small reorg (pandas-dev#23587)
  CI: Check in the CI that assert_raises_regex is not being used (pandas-dev#23627)
  CLN:Remove unused **kwargs from user facing methods (pandas-dev#23249)
  DOC: Enhancing pivot / reshape docs (pandas-dev#21038)
  TST: Fix xfailing DataFrame arithmetic tests by transposing (pandas-dev#23620)
* upstream/master: (25 commits)
  DOC: Delete trailing blank lines in docstrings. (pandas-dev#23651)
  DOC: Change release and whatsnew (pandas-dev#21599)
  DOC: Fix format of the See Also descriptions (pandas-dev#23654)
  DOC: update pandas.core.groupby.DataFrameGroupBy.resample docstring. (pandas-dev#20374)
  ENH: Allow export of mixed columns to Stata strl (pandas-dev#23692)
  CLN: Remove unnecessary code (pandas-dev#23696)
  Pin flake8-rst version (pandas-dev#23699)
  Implement _most_ of the EA interface for DTA/TDA (pandas-dev#23643)
  CI: raise clone depth limit on CI
  BUG: Fix Series/DataFrame.rank(pct=True) with more than 2**24 rows (pandas-dev#23688)
  REF: Move Excel names parameter handling to CSV (pandas-dev#23690)
  DOC: Accessing files from a S3 bucket. (pandas-dev#23639)
  Fix errorbar visualization (pandas-dev#23674)
  DOC: Surface / doc mangle_dupe_cols in read_excel (pandas-dev#23678)
  DOC: Update is_sparse docstring (pandas-dev#19983)
  BUG: Fix read_excel w/parse_cols & empty dataset (pandas-dev#23661)
  Add to_flat_index method to MultiIndex (pandas-dev#22866)
  CLN: Move to_excel to generic.py (pandas-dev#23656)
  TST: IntervalTree.get_loc_interval should return platform int (pandas-dev#23660)
  CI: Allow to compile docs with ipython 7.11 pandas-dev#22990 (pandas-dev#23655)
  ...
@@ -961,6 +962,7 @@ Other API Changes
- :class:`DateOffset` attribute `_cacheable` and method `_should_cache` have been removed (:issue:`23118`)
- Comparing :class:`Timedelta` to be less or greater than unknown types now raises a ``TypeError`` instead of returning ``False`` (:issue:`20829`)
- :meth:`Index.hasnans` and :meth:`Series.hasnans` now always return a python boolean. Previously, a python or a numpy boolean could be returned, depending on circumstances (:issue:`23294`).
- :func:The order of the arguments of `DataFrame.to_html` and `DataFrame.to_string` is rearranged to be consistent with each other. (:issue:`23614`)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is malformed here, you need the :func: to go with the DataFrame.to_html and .to_string (separate ones)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Done.

* upstream/master:
  BUG: to_html misses truncation indicators (...) when index=False (pandas-dev#22786)
  API/DEPR: replace "raise_conflict" with "errors" for df.update (pandas-dev#23657)
  BUG: Append DataFrame to Series with dateutil timezone (pandas-dev#23685)
  CLN/CI: Catch that stderr-warning! (pandas-dev#23706)
  ENH: Allow for join between two multi-index dataframe instances (pandas-dev#20356)
  Ensure Index._data is an ndarray (pandas-dev#23628)
  DOC: flake8-per-pr for windows users (pandas-dev#23707)
  DOC: Handle exceptions when computing contributors. (pandas-dev#23714)
  DOC: Validate space before colon docstring parameters pandas-dev#23483 (pandas-dev#23506)
  BUG-22984 Fix truncation of DataFrame representations (pandas-dev#22987)
@jreback jreback merged commit a23f901 into pandas-dev:master Nov 15, 2018
@jreback
Copy link
Contributor

jreback commented Nov 15, 2018

thanks

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018
@thoo thoo deleted the to_html-to_string branch January 2, 2019 20:26
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Docs IO HTML read_html, to_html, Styler.apply, Styler.applymap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLN: Consistency of arguments in to_html and to_string
4 participants