BUG: Iteration over DatetimeIndex stops at chunksize (GH21012)
State: closed

- [x] closes #21012
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

avoided using `yield` and changing DatetimeIndex itself into a iterator. @cbertinato 's advice was helpful.

Labels:
name: Bug

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21027?src=pr&el=h1) Report
> Merging [#21027](https://codecov.io/gh/pandas-dev/pandas/pull/21027?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/d007c2dff6fa1164ea3d208731af1ac5bdbc01b8?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `96.66%`.

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

```diff
@@            Coverage Diff             @@
##           master   #21027      +/-   ##
==========================================
- Coverage   91.82%   91.82%   -0.01%     
==========================================
  Files         153      153              
  Lines       49502    49523      +21     
==========================================
+ Hits        45457    45475      +18     
- Misses       4045     4048       +3
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.22% <96.66%> (-0.01%)` | :arrow_down: |
| #single | `41.9% <96.66%> (+0.02%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/21027?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/21027/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.76% <96.66%> (-0.02%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/21027/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `84.38% <0%> (-0.21%)` | :arrow_down: |
| [pandas/io/parsers.py](https://codecov.io/gh/pandas-dev/pandas/pull/21027/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnNlcnMucHk=) | `95.46% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21027?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/21027?src=pr&el=footer). Last update [d007c2d...ec651d7](https://codecov.io/gh/pandas-dev/pandas/pull/21027?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Couple of comments:
- Think about changing the name of the iterator class to something a bit less general than `Iterator`.
- Reviewers will eventually ask, but you will need a whatsnew entry.
I'm not sure where to write a whatsnew (this is not a bug in 0.22.0).
Please tell me.
Good point on the whatsnew.  I suppose it's not necessary in this case.
Yes, whatsnew is not necessary as it was not a bug before.

IMO, this PR adds a lot of complexity, but probably necessary if we want to keep both:

- the optimization to iterate over chunks (https://github.com/pandas-dev/pandas/pull/7720)
- the removal of yield to please dill (https://github.com/pandas-dev/pandas/pull/18848)

I don't know how important either of those are (eg couldn't this be fixed in dill itself? Other pickle libraries have no problem with it?), so difficult to assess for me. 
If one of the above restrictions could be removed, we wouldn't need this added complexity of returning a custom iterator object
An alternative is the original fix proposed by @kittoku which entails making `DateTimeIndex` into an iterator and adding a condition in `from_tuples` to avoid the code breaking the test.  It is a kind of hack, but it does preclude the need to use a custom iterator.
So there are 4 options from what I see now.

1 : `DatetimeIndex.__iter__` returns `iter(self)`
**good** : the simplest , dill can pickle **bad** : low performance

2 : `DatetimeIndex.__iter__` returns generator using `yield` (like below).
**good** : simpler  **bad** : dill can't pickle
```
    # from GH7720
    def __iter__(self):
        """
        Return an iterator over the boxed values
        Returns
        -------
        Timestamps : ndarray
        """

        # convert in chunks of 10k for efficiency
        data = self.asi8
        l = len(self)
        chunksize = 10000
        chunks = int(l / chunksize) + 1
        for i in range(chunks):
            start_i = i*chunksize
            end_i = min((i+1)*chunksize,l)
            converted = tslib.ints_to_pydatetime(data[start_i:end_i], tz=self.tz, offset=self.offset, box=True)
            for v in converted:
            yield v
```

3 : `DatetimeIndex.__iter__` returns a separate iterator (like this PR).
**good** : not need to edit other codes , dill can pickle **bad** : complex, need to make a class

4 : `DatetimeIndex.__iter__` returns itself as a iterator and edit other codes (like below).
**good** : dill can pickle **bad** : complex
suggestions in #21012 and
```
#pandas\core\indexes\multi.py
def from_tuples(cls, tuples, sortorder=None, names=None):
    ...
    if not is_list_like(tuples):
        raise TypeError('Input must be a list / sequence of tuple-likes.')
    elif is_iterator(tuples): # add not isinstance(tuples, Index) here
        tuples = list(tuples)
```
```
#pandas/io/formats/printing.py
# exchange `if` and `elif` to catch DatetimeIndex as sequence before assumed as iterator
def pprint_thing(thing, _nest_lvl=0, escape_chars=None, default_escapes=False,
    ...
    if (compat.PY3 and hasattr(thing, '__next__')) or hasattr(thing, 'next'):
        return compat.text_type(thing)
    elif (isinstance(thing, dict) and
          _nest_lvl < get_option("display.pprint_nest_depth")):
        result = _pprint_dict(thing, _nest_lvl, quote_strings=True,
                              max_seq_items=max_seq_items)
    elif (is_sequence(thing) and
          _nest_lvl < get_option("display.pprint_nest_depth")):
        result = _pprint_seq(thing, _nest_lvl, escape_chars=escape_chars,
                             quote_strings=quote_strings,
                             max_seq_items=max_seq_items)
```

I'm not in need to pickle by dill, so option 2 is best to me. I thought option 3 was the second. But now option 4 looks not bad because an idea of DatetimeIndex as iterator seems not so strange.
sure this is a bug, but not a blocker for 0.23.0
It is odd of me to say that, but could we discard my PR and cancel a commit of #18848 (take option 2)?

I can't find a reason to pickle `DatetimeIndex.__iter__()` while we can pickle `DatetimeIndex`.
I want to hear others' opinions.
@kittoku that's a good point. We don't even have a test for that , and to be honest most folks use ``cloudpickle`` anyhow.
May I leave the test?
yes test is ok
thanks @kittoku 

yeah the original (not-released) patched wasn't doc-ced / tested, and unclear how this actually solves a problem, while breaking this.