ENH: consistency of input args for boundaries in DataFrame.between_time() #40245
State: closed

- [ ] xref #40245 
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry

Labels:
name: Enhancement
name: Compat
description: pandas objects compatability with Numpy or Python functions
name: Deprecate
description: Functionality to remove in pandas

Comments:
@jreback @attack68 Can you please take a look at this and approve the workflow. Also, let me know if any changes are required. Thanks!
Hello @suyashgupta01! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:







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

##### Comment last updated at 2021-09-10 14:17:04 UTC
@attack68 There's a small conflict in the doc/source/whatsnew/v1.4.0.rst
I can't resolve the conflict as I don't have the privilege to do so, can you please take a look at it.
> @attack68 There's a small conflict in the doc/source/whatsnew/v1.4.0.rst
> I can't resolve the conflict as I don't have the privilege to do so, can you please take a look at it.

you don't need privileges to resolve merge conflicts, see https://docs.github.com/en/github/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line
> > @attack68 There's a small conflict in the doc/source/whatsnew/v1.4.0.rst
> > I can't resolve the conflict as I don't have the privilege to do so, can you please take a look at it.
> 
> you don't need privileges to resolve merge conflicts, see https://docs.github.com/en/github/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line

Thank you, I'll follow this :)
@MarcoGorelli Is there a relatively straightforward way to run the CI checks locally before pushing commits here? I'd want to run docstring related checks (they failed in the most recent version of the PR).
I did read the contributing guide on this but it just gives a very high-level overview and no clear instructions are given.
check https://pandas.pydata.org/docs/development/contributing_documentation.html#updating-a-pandas-docstring
> check https://pandas.pydata.org/docs/development/contributing_documentation.html#updating-a-pandas-docstring

Thanks!
> Getting there 💪
> 
> There's a few docstring validation failures, can you fix them up please?

I'm facing an issue here. When I run `python scripts/validate_docstrings.py pandas.DataFrame.between_time`, it seems to be checking the docstring for the current version of pandas (the one on pandas-dev:master) and NOT my docstring. Can you please let me know how I can get it to check the docstring of my local function?

Here's the output:
```
################################################################################
################## Docstring (pandas.DataFrame.between_time)  ##################
################################################################################

Select values between particular times of the day (e.g., 9:00-9:30 AM).

By setting ``start_time`` to be later than ``end_time``,
you can get the times that are *not* between the two times.

Parameters
----------
start_time : datetime.time or str
    Initial time as a time filter limit.
end_time : datetime.time or str
    End time as a time filter limit.
include_start : bool, default True
    Whether the start time needs to be included in the result.
include_end : bool, default True
    Whether the end time needs to be included in the result.
axis : {0 or 'index', 1 or 'columns'}, default 0
    Determine range time on index or columns value.

Returns
-------
Series or DataFrame
    Data from the original object filtered to the specified dates range.

Raises
------
TypeError
    If the index is not  a :class:`DatetimeIndex`

See Also
--------
at_time : Select values at a particular time of the day.
first : Select initial periods of time series based on a date offset.
last : Select final periods of time series based on a date offset.
DatetimeIndex.indexer_between_time : Get just the index locations for
    values between particular times of the day.

Examples
--------
>>> i = pd.date_range('2018-04-09', periods=4, freq='1D20min')
>>> ts = pd.DataFrame({'A': [1, 2, 3, 4]}, index=i)
>>> ts
                     A
2018-04-09 00:00:00  1
2018-04-10 00:20:00  2
2018-04-11 00:40:00  3
2018-04-12 01:00:00  4

>>> ts.between_time('0:15', '0:45')
                     A
2018-04-10 00:20:00  2
2018-04-11 00:40:00  3

You get the times that are *not* between two times by setting
``start_time`` later than ``end_time``:

>>> ts.between_time('0:45', '0:15')
                     A
2018-04-09 00:00:00  1
2018-04-12 01:00:00  4

################################################################################
################################## Validation ##################################
################################################################################

Docstring for "pandas.DataFrame.between_time" correct. :)
```
It should check your local version, so I guess there's something not quite right with your environment. But you can check the CI logs:
```
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR10:pandas.DataFrame.between_time:Parameter "inclusive" requires a space before the colon separating the parameter name and type
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.DataFrame.between_time:Parameter ""neither"" has no type
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.DataFrame.between_time:Parameter ""left"" has no type
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.DataFrame.between_time:Parameter ""right"}" has no type
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.DataFrame.between_time:Parameter "default "both"" has no type
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR10:pandas.Series.between_time:Parameter "inclusive" requires a space before the colon separating the parameter name and type
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.Series.between_time:Parameter ""neither"" has no type
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.Series.between_time:Parameter ""left"" has no type
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.Series.between_time:Parameter ""right"}" has no type
Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.Series.between_time:Parameter "default "both"" has no type
```

Also, check the [docstring guide](https://pandas.pydata.org/pandas-docs/stable/development/contributing_docstring.html) for guidance
I'm a bit confused about this failing test: `codecov/patch — 33.33% of diff hit (target 50.00%)`.

What can we do to improve on this? Should I write more test cases?
yes, please check that your test cases cover all the lines of code you've changed
As suggested [here](https://github.com/pandas-dev/pandas/pull/43248#discussion_r699185320), I had reduced the `test_between_time_warn()` to a minimum as suggested by attack68. However, I think doing that leaves a gap from my side; the functionality of the `between_time()` is never tested with the old arguments (`include_start, include_end`) in other test cases. Maybe I can restore the complete test case here and update the PR, does that seem right?
Before that, start with tests which check that the correct errors are raised if invalid parameters are passed
> Before that, start with tests which check that the correct errors are raised if invalid parameters are passed

Ok I'll start with that :)
> It should check your local version, so I guess there's something not quite right with your environment. But you can check the CI logs:
> 
> ```
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR10:pandas.DataFrame.between_time:Parameter "inclusive" requires a space before the colon separating the parameter name and type
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.DataFrame.between_time:Parameter ""neither"" has no type
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.DataFrame.between_time:Parameter ""left"" has no type
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.DataFrame.between_time:Parameter ""right"}" has no type
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.DataFrame.between_time:Parameter "default "both"" has no type
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR10:pandas.Series.between_time:Parameter "inclusive" requires a space before the colon separating the parameter name and type
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.Series.between_time:Parameter ""neither"" has no type
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.Series.between_time:Parameter ""left"" has no type
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.Series.between_time:Parameter ""right"}" has no type
> Error: /home/runner/work/pandas/pandas/pandas/core/generic.py:7605:PR04:pandas.Series.between_time:Parameter "default "both"" has no type
> ```
> 
> Also, check the [docstring guide](https://pandas.pydata.org/pandas-docs/stable/development/contributing_docstring.html) for guidance

You're right, something's wrong with my docker container, but I don't know what. Many test cases are also failing for the code that I never even touched (code unrelated to this PR). This error is popping up: `AttributeError: 'BlockManager' object has no attribute '_rebuild_blknos_and_blklocs'` for lines that look like this
```
if frame_or_series is not DataFrame:
>           ts = ts[0]
```

Can you let me know, is there anything that can be done? I did rebuild the container, but that doesn't seem to help.

Edit: I'll go ahead and set up the whole environment from scratch. 

Edit: Reinstalling everything in a new docker container fixes the issue.
There's a deprecate directive in [`to_excel` (for the argument `engine`) in generic.py](https://github.com/pandas-dev/pandas/blob/master/pandas/core/generic.py#L2133). However, that doesn't seem to render in VS Code. I'm not sure if this is some sort of a bug. 

I originally wanted to format my own `deprecate` directive for `include_start, include_end` using that as an example, but there's already a problem there. Can anyone please take a look at this?

> There's a deprecate directive in [`to_excel` (for the argument `engine`) in generic.py](https://github.com/pandas-dev/pandas/blob/master/pandas/core/generic.py#L2133). However, that doesn't seem to render in VS Code. I'm not sure if this is some sort of a bug.
> 
> I originally wanted to format my own `deprecate` directive for `include_start, include_end` using that as an example, but there's already a problem there. Can anyone please take a look at this?

The way to check if it renders is by generating the docstring - the deprecation in `to_excel` looks fine if you do that. Run the script from https://pandas.pydata.org/docs/development/contributing_documentation.html#updating-a-pandas-docstring and check the docs page in your browser
> > There's a deprecate directive in [`to_excel` (for the argument `engine`) in generic.py](https://github.com/pandas-dev/pandas/blob/master/pandas/core/generic.py#L2133). However, that doesn't seem to render in VS Code. I'm not sure if this is some sort of a bug.
> > I originally wanted to format my own `deprecate` directive for `include_start, include_end` using that as an example, but there's already a problem there. Can anyone please take a look at this?
> 
> The way to check if it renders is by generating the docstring - the deprecation in `to_excel` looks fine if you do that. Run the script from https://pandas.pydata.org/docs/development/contributing_documentation.html#updating-a-pandas-docstring and check the docs page in your browser

As I'd highlighted previously, this script doesn't seem to check the current local docstring. I'm not sure how can I force it do that.

```
(base) root@63c891769b8e:/workspaces/pandas# python scripts/validate_docstrings.py pandas.DataFrame.between_time

################################################################################
################## Docstring (pandas.DataFrame.between_time)  ##################
################################################################################

Select values between particular times of the day (e.g., 9:00-9:30 AM).

By setting ``start_time`` to be later than ``end_time``,
you can get the times that are *not* between the two times.

Parameters
----------
start_time : datetime.time or str
    Initial time as a time filter limit.
end_time : datetime.time or str
    End time as a time filter limit.
include_start : bool, default True
    Whether the start time needs to be included in the result.
include_end : bool, default True
    Whether the end time needs to be included in the result.
axis : {0 or 'index', 1 or 'columns'}, default 0
    Determine range time on index or columns value.

Returns
-------
Series or DataFrame
    Data from the original object filtered to the specified dates range.

Raises
------
TypeError
    If the index is not  a :class:`DatetimeIndex`

See Also
--------
at_time : Select values at a particular time of the day.
first : Select initial periods of time series based on a date offset.
last : Select final periods of time series based on a date offset.
DatetimeIndex.indexer_between_time : Get just the index locations for
    values between particular times of the day.

Examples
--------
>>> i = pd.date_range('2018-04-09', periods=4, freq='1D20min')
>>> ts = pd.DataFrame({'A': [1, 2, 3, 4]}, index=i)
>>> ts
                     A
2018-04-09 00:00:00  1
2018-04-10 00:20:00  2
2018-04-11 00:40:00  3
2018-04-12 01:00:00  4

>>> ts.between_time('0:15', '0:45')
                     A
2018-04-10 00:20:00  2
2018-04-11 00:40:00  3

You get the times that are *not* between two times by setting
``start_time`` later than ``end_time``:

>>> ts.between_time('0:45', '0:15')
                     A
2018-04-09 00:00:00  1
2018-04-12 01:00:00  4

################################################################################
################################## Validation ##################################
################################################################################

Docstring for "pandas.DataFrame.between_time" correct. :)
```
Sorry, it's this one to build the docs: https://pandas.pydata.org/docs/development/contributing_documentation.html#building-the-documentation
Also, with regards to validating docstrings, can you show the output of
```
$ python
>>> import pandas
>>> pandas.__version__
```
from within your environment?
> Also, with regards to validating docstrings, can you show the output of
> 
> ```
> $ python
> >>> import pandas
> >>> pandas.__version__
> ```
> 
> from within your environment?

>>> import pandas
>>> pandas.__version__
'1.4.0.dev0+666.gbb613534c2.dirty'
![image](https://user-images.githubusercontent.com/45630098/132225039-4937abc4-f136-4874-83d3-106dc2083772.png)
Docstring seems to render fine on my side after building docs :) @attack68 
Cool, thanks - couple of `mypy` errors left to resolve
```
pandas/core/generic.py:7714: error: Argument 1 to "int" has incompatible type "Union[bool, NoDefault]"; expected "Union[str, bytes, SupportsInt, SupportsIndex, _SupportsTrunc]"  [arg-type]
pandas/core/generic.py:7715: error: Argument 1 to "int" has incompatible type "Union[bool, NoDefault]"; expected "Union[str, bytes, SupportsInt, SupportsIndex, _SupportsTrunc]"  [arg-type]
Found 2 errors in 1 file (checked 1312 source files)
```

I _think_ you can get around this by doing
```diff
- left = 1 if include_start == lib.no_default else int(include_start)
+ left = 1 if isinstance(include_start, lib.NoDefault) else int(include_start)
```
`pandas-dev.pandas Failing after 60m — Build #20210906.14` was cancelled after 60 minutes as that's the most time it's allowed to run. Not sure what I can do about this. Can anyone please take a look?
https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=65658&view=results
@MarcoGorelli can you approve the workflows, please?

@jreback It's all green, take a look :)
@jreback Are any other changes required?
lgtm @attack68 @MarcoGorelli if any comments.
@MarcoGorelli Made the requested changes, please take a look :)