ENH: allow using '+' sign in the argument to `to_offset()` #18171

frexvahi · 2017-11-08T13:54:28Z

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

frexvahi · 2017-11-08T13:56:33Z

I made the change here in frequencies.pxd, which presumably means that now frequencies can also be specified with a '+' sign. Maybe this is not desired, in which case I could include a modified version of the opattern regexp in the to_offset function itself.

frexvahi · 2017-11-08T13:59:57Z

Should the whatsnew entry go in "Other API changes"?

frexvahi · 2017-11-08T14:02:48Z

Also, does this need a separate issue?

TomAugspurger

This can go under "Other Enhancements" and you can use 18171 for the issue number. No need for a separate issue.

TomAugspurger · 2017-11-08T16:02:23Z

pandas/tseries/frequencies.py

-        except Exception:
-            raise ValueError(_INVALID_FREQ_ERROR.format(freq))
+        except Exception as e:
+            raise ValueError(_INVALID_FREQ_ERROR.format(freq)) from e


I don't think this is valid syntax in python 2

TomAugspurger · 2017-11-08T16:05:23Z

pandas/tests/tseries/test_frequencies.py

@@ -169,6 +169,15 @@ def test_to_offset_leading_zero(self):
        result = frequencies.to_offset(freqstr)
        assert (result.n == -194)

+    def test_to_offset_leading_plus(self):


Could you add a couple invalid freqstrs, like '+-1d', '-+1d', '+1', '+d' and ensure that they all raise.

At least I think they should all raise? d is apparently valid, but -d isn't.

codecov · 2017-11-09T17:03:51Z

Codecov Report

Merging #18171 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #18171      +/-   ##
==========================================
- Coverage   91.42%   91.38%   -0.05%     
==========================================
  Files         163      163              
  Lines       50064    50068       +4     
==========================================
- Hits        45772    45755      -17     
- Misses       4292     4313      +21

Flag	Coverage Δ
#multiple	`89.19% <100%> (-0.03%)`	⬇️
#single	`40.33% <0%> (-0.09%)`	⬇️

Impacted Files	Coverage Δ
pandas/tseries/frequencies.py	`96% <100%> (ø)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/plotting/_converter.py	`63.38% <0%> (-1.82%)`	⬇️
pandas/core/frame.py	`97.8% <0%> (-0.1%)`	⬇️
pandas/core/generic.py	`95.72% <0%> (ø)`	⬆️
pandas/io/formats/format.py	`96.01% <0%> (ø)`	⬆️
pandas/core/config_init.py	`98.34% <0%> (ø)`	⬆️
pandas/io/parquet.py	`65.38% <0%> (ø)`	⬆️
pandas/compat/__init__.py	`58.1% <0%> (ø)`	⬆️
pandas/core/groupby.py	`92.04% <0%> (+0.01%)`	⬆️

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 17e0b13...71bd8db. Read the comment docs.

codecov · 2017-11-09T17:04:06Z

Codecov Report

❗ No coverage uploaded for pull request base (master@a6345c7). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #18171   +/-   ##
=========================================
  Coverage          ?   91.38%           
=========================================
  Files             ?      163           
  Lines             ?    50064           
  Branches          ?        0           
=========================================
  Hits              ?    45751           
  Misses            ?     4313           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`89.19% <ø> (?)`
#single	`40.36% <ø> (?)`

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 a6345c7...5d0b363. Read the comment docs.

jreback

small doc change. ping on green.

jreback · 2017-11-10T13:57:15Z

doc/source/whatsnew/v0.22.0.txt

@@ -23,7 +23,7 @@ Other Enhancements
 ^^^^^^^^^^^^^^^^^^

 - Better support for ``Dataframe.style.to_excel()`` output with the ``xlsxwriter`` engine. (:issue:`16149`)
-
+- ``pd.tseries.frequencies.to_offset()`` now accepts '+' signs e.g. '+1h'. (:issue:`18171`)


use a :func:`pandas.tseries.frequencies.to_offset` here

say accepts leading '+' signs

jreback · 2017-11-10T14:10:21Z

doc/source/whatsnew/v0.22.0.txt

@@ -23,7 +23,7 @@ Other Enhancements
 ^^^^^^^^^^^^^^^^^^

 - Better support for ``Dataframe.style.to_excel()`` output with the ``xlsxwriter`` engine. (:issue:`16149`)
-
+- :func:`pd.tseries.frequencies.to_offset()` now accepts leading '+' signs e.g. '+1h'. (:issue:`18171`)


I think this needs to be pandas (and not pd)

The other entries in the file all say 'pd' and not 'pandas'. In previous whatsnew.txt files there seems to be a mixture but still a preponderance of 'pd'

hmm, then maybe this works. ok.

jreback · 2017-11-10T14:25:05Z

thanks @frexvahi

…v#18171)

TomAugspurger reviewed Nov 8, 2017

View reviewed changes

TomAugspurger added this to the 0.22.0 milestone Nov 8, 2017

TomAugspurger added Frequency DateOffsets Datetime Datetime data dtype API Design labels Nov 8, 2017

frexvahi added 2 commits November 9, 2017 17:52

Allow using '+' sign in the argument to to_offset()

8303bb2

Added tests that invalid frequencies are recognized as invalid

9d104c5

frexvahi force-pushed the allow-plus-sign-in-to_offset branch from b20de97 to 71bd8db Compare November 9, 2017 17:03

frexvahi force-pushed the allow-plus-sign-in-to_offset branch from 71bd8db to 2d8763a Compare November 9, 2017 17:05

frexvahi changed the title ~~Allow using '+' sign in the argument to to_offset()~~ ENH: allow using '+' sign in the argument to to_offset() Nov 10, 2017

jreback requested changes Nov 10, 2017

View reviewed changes

Describe changes in whatsnew.txt

5d0b363

frexvahi force-pushed the allow-plus-sign-in-to_offset branch from 2d8763a to 5d0b363 Compare November 10, 2017 14:05

jreback reviewed Nov 10, 2017

View reviewed changes

jreback approved these changes Nov 10, 2017

View reviewed changes

jreback merged commit 75c1fa0 into pandas-dev:master Nov 10, 2017

frexvahi deleted the allow-plus-sign-in-to_offset branch November 10, 2017 14:25

No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017

ENH: allow using '+' sign in the argument to to_offset() (pandas-de…

49e6c8d

…v#18171)

Uh oh!

ENH: allow using '+' sign in the argument to to_offset() #18171

ENH: allow using '+' sign in the argument to to_offset() #18171

Uh oh!

Conversation

frexvahi commented Nov 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frexvahi commented Nov 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frexvahi commented Nov 8, 2017

Uh oh!

frexvahi commented Nov 8, 2017

Uh oh!

TomAugspurger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomAugspurger Nov 8, 2017

Choose a reason for hiding this comment

Uh oh!

TomAugspurger Nov 8, 2017

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 9, 2017

Codecov Report

Uh oh!

codecov bot commented Nov 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jreback left a comment

Choose a reason for hiding this comment

Uh oh!

jreback Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

frexvahi Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

jreback Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

frexvahi Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

jreback Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

jreback commented Nov 10, 2017

Uh oh!

Uh oh!

ENH: allow using '+' sign in the argument to `to_offset()` #18171

ENH: allow using '+' sign in the argument to `to_offset()` #18171

frexvahi commented Nov 8, 2017 •

edited

Loading

frexvahi commented Nov 8, 2017 •

edited

Loading

TomAugspurger left a comment •

edited

Loading

codecov bot commented Nov 9, 2017 •

edited

Loading