Fix Timestamp rounding
State: closed

- Closes #21262
- Tests added (thanks  @Safrone [PR](https://github.com/pandas-dev/pandas/pull/21265))

This change-set is to avoid rounding a timestamp when the timestamp is a multiple of the frequency string passed in.

"Values" param passed into round_ns can either be a np array or int. So relevant handling added for both.

FYI I havn't used Cython much before so keen to get peoples thoughts/feedback.

Thanks


Labels:
name: Datetime
description: Datetime data dtype
name: Regression
description: Functionality that used to work in a prior pandas version

Comments:
Hello @alimcmaster1! Thanks for updating the PR.





Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on June 28, 2018 at 20:01 Hours UTC
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21507?src=pr&el=h1) Report
> Merging [#21507](https://codecov.io/gh/pandas-dev/pandas/pull/21507?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/e0f978d43c960e0af7944a9d7d410662e071c0c7?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/21507/graphs/tree.svg?width=650&height=150&token=eZ4WkYLtcO&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/21507?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #21507   +/-   ##
=======================================
  Coverage    91.9%    91.9%           
=======================================
  Files         154      154           
  Lines       49555    49555           
=======================================
  Hits        45542    45542           
  Misses       4013     4013
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.27% <ø> (ø)` | :arrow_up: |
| #single | `42.03% <ø> (ø)` | :arrow_up: |


------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21507?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21507?src=pr&el=footer). Last update [e0f978d...2d0fa8b](https://codecov.io/gh/pandas-dev/pandas/pull/21507?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

A few thoughts:

1) With your solution, I think it's better to make `round_ns` only take arrays (therefore wrap scalar inputs as array) to avoid checking the type of the input (and moreover `cdef`ing them) and iterating over values. It appears the solution can still work by computing `values % unit` as a boolean array and operating with that. We do something similar with `tz_localize_to_utc`.

2) Include tests for `DatetimeIndex.round`

3) Looks like there are some linting errors (your spacing seems to be 2 instead of 4 in some places).
Thanks @mroeschke for the comments. I've done (2) and (3) added a few additional test cases to the DateTimeIndex ceil/floor and one that clearly shows how round should behave for this bug.

1) Yes this definitely makes sense. Just to clarify are you thinking in 
`_round` we check the type of `value` if its an int we wrap in an np array. Hence we can cdef `round_ns`?

Best,

Alistair


Rather have `round_ns` (which we can at best `cpdef` since it's imported by a python file) _only_ accept numpy arrays. So since this method is used by timestamps, we would have to wrap it's `self.value` input as a numpy array. The relevant rounding code for `Timestamp`s would turn into:

```
def _round(self, freq, rounder):
    if self.tz is not None:
        value = self.tz_localize(None).value
    else:
        value = self.value
    value = np.array([value], dtype=np.int64)
    r = round_ns(value, rounder, freq)
```
So it shows up on the issue properly: #21262 
Cleaned up my implementation here
1) Params added in test cases
2) round_ns now only takes np.array ( hence cleaned up the logic in here )

Thoguhts @mroeschke ?
Thanks for the revision. As @jreback mentions, I dont think it's necessary to iterate, You should be able to perform the adjustments with vectorization. At a high level the logic should look like:

```
mask = value % unit == 0
if mask.all():
    return value
values[~mask] = _round_non_int_multiple(values[~mask])
return values
```
That @mroeschke your logic above seems much neater let me refactor!

Thanks @jreback, ive updated, let me know if there is anything else!
Done! @jreback
Travis flagged some linting errors:

```
pandas/_libs/tslibs/timestamps.pyx:92:14: E128 continuation line under-indented for visual indent
pandas/_libs/tslibs/timestamps.pyx:92:80: E501 line too long (80 > 79 characters)
pandas/_libs/tslibs/timestamps.pyx:103:80: E501 line too long (87 > 79 characters)
pandas/_libs/tslibs/timestamps.pyx:104:14: E128 continuation line under-indented for visual indent
```
thanks @alimcmaster1 
thanks @jreback and @mroeschke for helping review!