[EHN] pandas.DataFrame.to_orc
State: closed

- [x] closes #43864
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit) for how to run them
- [x] whatsnew entry


Labels:
name: Enhancement
name: IO Data
description: IO issues that don't fit into a more specific label
name: Arrow
description: pyarrow functionality

Comments:
Hello @iajoiner! 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 2022-06-13 21:37:24 UTC
Thanks @NickFillot for your effort in integrating my ORC writer into Pandas. Now since your PR has been closed I will take care of modifying it and getting it approved from now on.

Here is the link to Nick's PR which I unfortunately can not reopen: https://github.com/pandas-dev/pandas/pull/43860
Since https://github.com/apache/arrow/pull/9702 will significantly add to the ORC writer API shall we delay the merge until late Jan when Arrow 7.0.0 is released?
> Since apache/arrow#9702 will significantly add to the ORC writer API shall we delay the merge until late Jan when Arrow 7.0.0 is released?

I can't judge this. Probably depends at least on 1) whether it would create inconsistencies (early implementation and then with the new pyarrow later) and on 2) how soon you want it (pandas 1.4 is scheduled for New Year's Eve, 1.5/2.0 will then probably be a year after that).
@twoertwein I don't think there will be inconsistencies. I'm really just adding optional writer arguments that can be `**kwargs` in Pandas. Yes I want it out there ASAP.
@github-actions pre-commit
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
I’m still around. Will update this PR this month. It’s just that I have a few Arrow tickets to deal with now.
love to have this, if you can merge master and address comments we can look again
I just got my detailed `writeoptions` update into Arrow. Now it is time to finish `to_orc`.
@twoertwein @pitrou  Sorry for the delay. It is ready for review again!
@github-actions pre-commit
@twoertwein Sorry for the delay. Now that I have figured out how to test pandas in my docker container I can certify that all ORC-related pytest tests have passed and that when I manually use it it feels just like Parquet and CSV. Note that there might be one weird error in one check that clearly has nothing to do with this PR.
@twoertwein There is one more decision I’d like to consult you about: Currently the ORC writer in pyarrow doesn’t handle dictionary type which correspond to categorical type in pandas.

Hence for dataframes with one or more categorical columns we probably need to do one of the following:
1. Raise ValueError and explain the issue in Arrow.
2. Convert categorical columns into string / object columns before proceeding. Maybe we can warn users about the issue.

On the Arrow side I can get dictionary type enabled in the ORC writer in 9.0.0 if necessary. However given our release schedules it will likely be slightly after the next Pandas release.

Since Pandas needs backward compatibility the choice we make here is likely to remain until 2.0.0. Which one do you think is more appropriate? Personally I’m inclined towards 2 since categorical columns are quite common and that it is quite natural to change the behavior once we improve other typing-related aspects of Pandas according to the roadmap such as adding string type and making null handling consistent.
> @twoertwein There is one more decision I’d like to consult you about

Sorry, I cannot help you with that. @jbrockmendel or @jreback might be able to help you.
> @twoertwein There is one more decision I’d like to consult you about: Currently the ORC writer in pyarrow doesn’t handle dictionary type which correspond to categorical type in pandas.
> 
> Hence for dataframes with one or more categorical columns we probably need to do one of the following:
> 
> 1. Raise ValueError and explain the issue in Arrow.
> 2. Convert categorical columns into string / object columns before proceeding. Maybe we can warn users about the issue.
> 
> On the Arrow side I can get dictionary type enabled in the ORC writer in 9.0.0 if necessary. However given our release schedules it will likely be slightly after the next Pandas release.
> 
> Since Pandas needs backward compatibility the choice we make here is likely to remain until 2.0.0. Which one do you think is more appropriate? Personally I’m inclined towards 2 since categorical columns are quite common and that it is quite natural to change the behavior once we improve other typing-related aspects of Pandas according to the roadmap such as adding string type and making null handling consistent.

ok can you add a tests which fails for this case. i wouldn't worry too much here about requiring a higher arrow release, we do this all the time for various types. e.g. when its available in arrow we can add it for the next pandas release, nbd.
@jreback I have fixed the code according to your review with the exception of putting the `pytest.importorskip` decorator on the function for it might cause the decorator to be applied to the entire module (see https://github.com/pytest-dev/pytest/issues/9542 and https://github.com/pytest-dev/pytest/issues/9548) which is not desirable behavior because the ORC reader tests do need to run when pyarrow version is below 7.

The ASV Benchmarks error is a transient issue unrelated to the PR. Everything else is green (or will be an hour later). Please review.
Just made very slight changes to the docs in order to rerun the tests and eliminate the transient red.
@jreback After two retries now it is green.
@jreback Everything is green. Please review again. Thanks!
@mroeschke lgtm
@mroeschke Really thanks for reviewing this PR! I have one question about What’s New. Does the ORC writer qualify as a major enhancement? Personally I think it does and is the 4th most important major enhancement in 1.5.0.
> @mroeschke Really thanks for reviewing this PR! I have one question about What’s New. Does the ORC writer qualify as a major enhancement? Personally I think it does and is the 4th most important major enhancement in 1.5.0.

Sure! I think the whatsnew entry can be a small section instead of a one line mention
@mroeschke The only red is likely transient and completely unrelated to the PR. What's new has been updated. 

The only issue you pointed out which I haven't fixed is the PyArrow error issue since unsupport dtypes often cause segfaults in pyarrow. I have filed an Arrow ticket to fix it here: https://issues.apache.org/jira/browse/ARROW-16817 . Meanwhile due to the fact that Pandas 1.5.0 will be released before Arrow 9.0.0 in order not to have segfault when users do try to use unsupported dtypes can we use a filter in Pandas right now? We can then change it once the Arrow issue is fixed.

> @mroeschke The only red is likely transient and completely unrelated to the PR. What's new has been updated.
> 
> The only issue you pointed out which I haven't fixed is the PyArrow error issue since unsupport dtypes often cause segfaults in pyarrow. I have filed an Arrow ticket to fix it here: https://issues.apache.org/jira/browse/ARROW-16817 . Meanwhile due to the fact that Pandas 1.5.0 will be released before Arrow 9.0.0 in order not to have segfault when users do try to use unsupported dtypes can we use a filter in Pandas right now? We can then change it once the Arrow issue is fixed.

Sure sounds good. Can use manual dtype filtering for now with some corrections in my review.

Could you also uncommit `pandas/tests/io/data/orc/TestOrcFile.testReadWrite.orc`?
@mroeschke Please review again. All issues have been taken care of with the exception of using single quotes for msg which is not possible due to black.

P.S. Red stuff is unrelated to the PR.
@mroeschke Fixed haha. Really thanks for reviewing! Is the next step merging once it is clear that no error related to the PR exists? :)
Awesome thanks for the responsiveness down stretch!