BUG: to_clipboard() incorrectly claims I have specified a non-single character separator in excel mode.

[simonbance]

• I have checked that this issue has not already been reported.

• I have confirmed this bug exists on the latest version of pandas.

• I have confirmed this bug exists on the master branch of pandas.

Reproducible Example

import pandas as pd
df = pd.DataFrame({'numbers': [1, 2, 3], 'colors': ['red', 'white', 'blue']})
df.to_clipboard(index_names=False, header=False)

Gives the following trace.

C:\ProgramData\Miniforge3\lib\site-packages\pandas\io\clipboards.py:133: UserWarning: to_clipboard in excel mode requires a single character separator.
  warnings.warn(

Issue Description

The same error occurs regardless of whether I do not specify a separator, or if I include sep="," as an argument. In both cases, I am not specifying a separator of more than 1 character and there should be no problem passing this to to_csv().

Inspecting the source code at https://github.com/pandas-dev/pandas/blob/master/pandas/io/clipboards.py it appears we are correctly being caught by the if excel statement on line 120 (since we are in excel mode by default) but the try/except wrapper around the call to to_csv() is catching a TypeError. Further inspection is needed to identify what is causing the exception.

Tested in both pandas v1.2.3 and latest version v1.3.4.
I'm using miniconda on Windows 10.

Expected Behavior

The dataframe should be copied to the clipboard as string text without an exception being caught.

Installed Versions

INSTALLED VERSIONS

commit : 945c9ed
python : 3.8.12.final.0
python-bits : 64
OS : Windows
OS-release : 10
Version : 10.0.18363
machine : AMD64
processor : Intel64 Family 6 Model 142 Stepping 12, GenuineIntel
byteorder : little
LC_ALL : None
LANG : en

pandas : 1.3.4
numpy : 1.21.2
pytz : 2021.3
dateutil : 2.8.2
pip : 21.3
setuptools : 58.2.0
Cython : None
pytest : None
hypothesis : None
sphinx : 4.2.0
blosc : None
feather : None
xlsxwriter : None
lxml.etree : 4.6.3
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : 3.0.2
IPython : 7.28.0
pandas_datareader: None
bs4 : 4.10.0
bottleneck : None
fsspec : 2021.10.1
fastparquet : None
gcsfs : None
matplotlib : 3.4.3
numexpr : None
odfpy : None
openpyxl : None
pandas_gbq : None
pyarrow : None
pyxlsb : None
s3fs : None
scipy : 1.7.1
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : None
xlwt : None
numba : None

[simonbance]

Part of the issue here might be coding best practice; there are multiple statements within the try/except, and this makes it difficult to understand what is wrong. I'm going to stick my neck out and suggest that there should be no try/except here. If the problem really is with to_csv() then let to_csv() throw us an error that we can get useful information from. I don't know if trying to catch a TypeError is the correct approach here.

[fangchenli]

Part of the issue here might be coding best practice; there are multiple statements within the try/except, and this makes it difficult to understand what is wrong. I'm going to stick my neck out and suggest that there should be no try/except here. If the problem really is with to_csv() then let to_csv() throw us an error that we can get useful information from. I don't know if trying to catch a TypeError is the correct approach here.

Thanks for the feedback. The clipboard code was vendored from https://github.com/asweigart/pyperclip due to historical reasons, and it's not well tested. There is a lot of obvious improvement that can be made there. But modifying a vendored code too much is something we try to avoid.

This might be fixed on the latest pyperclip release. We could vendor the newer version or just use it as a dependency. I'll look at it during the weekend.

[simonbance]

Hi @fangchenli thanks for responding to my issue. I appreciate the explanation.
So you're saying that this is likely an error with pypaperclip. I wonder if the try/except that surrounds that part of the code should be modified, so that everything that's currently on lines 120 to 128 should be outside.
Perhaps we can make a separate check for a valid "sep" argument before calling to_csv(), not enclose it in the same try/except as the presumed problematic call to pypaperclip. At the moment the warning message seen by users is confusing and doesn't correctly reflect the nature of the issue.

Maybe that's something I could volunteer for, as a "good first issue".

[fangchenli]

@simonbance

I made this modification and run your example.

        except TypeError as err:
            warnings.warn(
                f"to_clipboard in excel mode requires a single character separator. {err}"
            )

I got this

/Users/fangchenli/Workspace/pandas-fangchenli/pandas/io/clipboards.py:135: UserWarning: to_clipboard in excel mode requires a single character separator. to_csv() got an unexpected keyword argument 'index_names'

[simonbance]

Actually I was thinking something like this (sorry can't test it right now):

    if excel:

        buf = StringIO()
            
        if sep is None:
            sep = "\t"
        elif len(sep) > 1:
            warnings.warn("to_clipboard in excel mode requires a single character separator.")
            # Do something appropriate here! Maybe set sep = "\t" or slice sep to length 1 ? (would be unpredictable!)
            # .....

        # clipboard_set (pyperclip) expects unicode
        obj.to_csv(buf, sep=sep, encoding="utf-8", **kwargs)
        text = buf.getvalue()

        try:
            clipboard_set(text)
            return
        except:
            warnings.warn(
                "there was a problem calling clipboard_set (pyperclip)."
            )
    elif sep is not None:
        warnings.warn("to_clipboard with excel=False ignores the sep argument.")

Does that make sense? If the exception is caused by pyperclip then the catch and associated warning should be specific to that problem. We can check the suitability of the sep string separately by looking at the length (and maybe some additional checks if needed).