ENH: Add strings_as_fixed_length parameter for df.to_records() (#18146)
State: closed

This option changes DataFrame.to_records() dtype for string arrays
to 'Sx', where x is the length of the longest string, instead of 'O"

- [x] closes #18146
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Enhancement
name: Output-Formatting
description: __repr__ of pandas objects, to_string
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

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

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

```diff
@@            Coverage Diff             @@
##           master   #22229      +/-   ##
==========================================
+ Coverage   92.31%   92.31%   +<.01%     
==========================================
  Files         166      166              
  Lines       52391    52426      +35     
==========================================
+ Hits        48363    48397      +34     
- Misses       4028     4029       +1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.73% <100%> (ø)` | :arrow_up: |
| #single | `42.82% <17.24%> (-0.23%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22229?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/22229/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `96.95% <100%> (+0.04%)` | :arrow_up: |
| [pandas/core/dtypes/inference.py](https://codecov.io/gh/pandas-dev/pandas/pull/22229/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2luZmVyZW5jZS5weQ==) | `98.46% <100%> (+0.07%)` | :arrow_up: |
| [pandas/core/arrays/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/22229/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3RpbWVkZWx0YXMucHk=) | `87.42% <0%> (-0.26%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/22229/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `87.68% <0%> (-0.1%)` | :arrow_down: |
| [pandas/core/arrays/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/22229/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3BlcmlvZC5weQ==) | `98.35% <0%> (-0.04%)` | :arrow_down: |
| [pandas/core/arrays/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/22229/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lcy5weQ==) | `97.91% <0%> (+0.17%)` | :arrow_up: |
| [pandas/core/arrays/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/22229/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lbGlrZS5weQ==) | `95.75% <0%> (+0.27%)` | :arrow_up: |

------

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

Thanks for the comments! Made the changes you requested.
@jreback Thanks for the feedback! I've made the changes. They are squashed into a new commit and rebased on top of upstream/master.
can you rebase
can you merge master
@gfyoung would you be able to merge master and update this
@jreback: Sure.  I’ll have time next week.
Hello @qinghao1! Thanks for updating the PR.







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

##### Comment last updated on December 29, 2018 at 02:01 Hours UTC
@jreback : I rebased and addressed all of the comments, including leveraging `infer_dtype` more extensively in the enhancement + adding a lot more tests.

That being said, I'm not 100% certain if the design decisions made were the best.  I commented pretty extensively in the `frame.py` diff regarding what I did.
@jreback : Could you take a look at this?

Would need some feedback on some of the implementation choices I made here...
@jreback : Addressed your comments, and all is green again.  PTAL.
@jreback : Added more tests, and everything is finally all green again.  PTAL.
@TomAugspurger thoughts on this.
collections.abc.Mapping ensures that. Seems like exactly what we want here.

Messing with the various is_*_like methods makes me nervous.

On Fri, Dec 28, 2018 at 11:06 AM gfyoung <notifications@github.com> wrote:

> *@gfyoung* commented on this pull request.
> ------------------------------
>
> In pandas/core/frame.py
> <https://github.com/pandas-dev/pandas/pull/22229#discussion_r244365011>:
>
> > +
> +        index_len = len(index_names)
> +        formats = []
> +
> +        for i, v in enumerate(arrays):
> +            index = i
> +
> +            if index < index_len:
> +                dtype_mapping = index_dtypes
> +                name = index_names[index]
> +            else:
> +                index -= index_len
> +                dtype_mapping = column_dtypes
> +                name = self.columns[index]
> +
> +            if isinstance(dtype_mapping, dict):
>
> One thing though: is_dict_like does not guarantee existence of the
> __contains__ method, which is what my implementation also relies on.
>
> I could do some try-except logic to make it more generic, or I could make
> the is_dict_like check slightly stricter.
>
> Thoughts?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22229#discussion_r244365011>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIo4XYuy2E7pohfVAISp-Np7aMnRMks5u9k-jgaJpZM4VxcGz>
> .
>

FWIW, collections.abc.Mapping is *the* definition of dict-like in the
Python world.

On Fri, Dec 28, 2018 at 11:15 AM Jeff Reback <notifications@github.com>
wrote:

> *@jreback* commented on this pull request.
> ------------------------------
>
> In pandas/core/frame.py
> <https://github.com/pandas-dev/pandas/pull/22229#discussion_r244366683>:
>
> > +
> +        index_len = len(index_names)
> +        formats = []
> +
> +        for i, v in enumerate(arrays):
> +            index = i
> +
> +            if index < index_len:
> +                dtype_mapping = index_dtypes
> +                name = index_names[index]
> +            else:
> +                index -= index_len
> +                dtype_mapping = column_dtypes
> +                name = self.columns[index]
> +
> +            if isinstance(dtype_mapping, dict):
>
> I really hate to use other things because they are completely
> unmaintanable in the codebase, hey just pick your favorite thing to check
> (dict, collections.Mapping), oh and you need something else, just reinvent
> it.
>
> This is the reason we have the is_* things.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22229#discussion_r244366683>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIq0eEBMyVHCUczCsJStAAl1qhoU-ks5u9lGbgaJpZM4VxcGz>
> .
>

Unfortunately, isinstance checks on ABCs are relatively slow, so I don't
think it should be used everywhere.

On Fri, Dec 28, 2018 at 11:29 AM Tom Augspurger <tom.augspurger88@gmail.com>
wrote:

> FWIW, collections.abc.Mapping is *the* definition of dict-like in the
> Python world.
>
> On Fri, Dec 28, 2018 at 11:15 AM Jeff Reback <notifications@github.com>
> wrote:
>
>> *@jreback* commented on this pull request.
>> ------------------------------
>>
>> In pandas/core/frame.py
>> <https://github.com/pandas-dev/pandas/pull/22229#discussion_r244366683>:
>>
>> > +
>> +        index_len = len(index_names)
>> +        formats = []
>> +
>> +        for i, v in enumerate(arrays):
>> +            index = i
>> +
>> +            if index < index_len:
>> +                dtype_mapping = index_dtypes
>> +                name = index_names[index]
>> +            else:
>> +                index -= index_len
>> +                dtype_mapping = column_dtypes
>> +                name = self.columns[index]
>> +
>> +            if isinstance(dtype_mapping, dict):
>>
>> I really hate to use other things because they are completely
>> unmaintanable in the codebase, hey just pick your favorite thing to check
>> (dict, collections.Mapping), oh and you need something else, just reinvent
>> it.
>>
>> This is the reason we have the is_* things.
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub
>> <https://github.com/pandas-dev/pandas/pull/22229#discussion_r244366683>,
>> or mute the thread
>> <https://github.com/notifications/unsubscribe-auth/ABQHIq0eEBMyVHCUczCsJStAAl1qhoU-ks5u9lGbgaJpZM4VxcGz>
>> .
>>
>

Does anyone have thoughts on the API discussion for combining `index_dtypes` and `column_dtypes`?
@TomAugspurger : I actually commented about this earlier, but not seeing it here in the comments...weird...

I actually prefer to keep separate, in part because I suspect the use case in the wild  will largely focus on the columns, not the indices.  Thus, it makes sense to keep separate to facilitate this use case, while still making it possible to specify dtypes for the indices if so desired.

Also, I prefer to avoid nested dict to reduce complexity for end users (in the case when we pass in different dtypes for columns and index).
Fair enough.

On Fri, Dec 28, 2018 at 12:38 PM gfyoung <notifications@github.com> wrote:

> @TomAugspurger <https://github.com/TomAugspurger> : I actually commented
> about this earlier, but not seeing it here in the comments...weird...
>
> I actually prefer to keep separate, in part because I suspect the use case
> in the wild will largely focus on the columns, not the indices. Thus, it
> makes sense to keep separate to facilitate this use case, while still
> making it possible to specify dtypes for the indices if so desired.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22229#issuecomment-450406870>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIkK0L3_zTj-1nfoSCKnfRWaOVmeyks5u9mUPgaJpZM4VxcGz>
> .
>

@jreback @TomAugspurger : Managed to expand `is_dict_like` without issues, and I added a test in `to_records` to ensure that generic dict-like's are accepted.  PTAL.
@jreback : Added a test for the expanded `is_dict_like`, which is all green.  PTAL.
Thanks for the help, @qinghao1 !