pyperclip upgrade #28471

krey · 2019-09-16T21:49:44Z

closes to_clipboard has no effect in ipython (works in python) (linux) #22707
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pandas/io/clipboard/__init__.py

jbrockmendel · 2019-09-16T22:04:29Z

are there any tests to go with this?

krey · 2019-09-16T22:12:36Z

are there any tests to go with this?

We could add pyperclip's tests to pandas'.

I'm not sure of a specific way to test #22707

jbrockmendel · 2019-09-16T22:20:33Z

Looks like CI is failing because an Exception subclass was removed from pandas.io.clipboard.exceptions import PyperclipException

WillAyd · 2019-09-17T00:07:48Z

What's the reasoning behind moving everything out of submodules into one __init__? Also can you explain the PR you are undoing from the OP? Is that a revived package now?

krey · 2019-09-17T01:10:45Z

What's the reasoning behind moving everything out of submodules into one __init__?

I don't know why they changed pyperclip that way.

Also can you explain the PR you are undoing from the OP?

I don't mind doing a manual merge if needed, but I wasn't sure if it mattered.

Is that a revived package now?

Not to my knowledge.

krey · 2019-09-17T01:14:11Z

Honestly I'm not even sure why __init__.py isn't just

from pyperclip import copy, paste
clipboard_get = paste
clipboard_set = copy

or something to that effect.

Is it to reduce the number of dependencies?

Or purely historical reasons?

WillAyd · 2019-09-17T03:11:02Z

Have to excuse my ignorance on this part of the codebase - I wasn't even aware we vendored this :-) OK I think I'm on board with this, since it's just an update to 1.7.0.

Can you check if this also solves #11720 ? cc @datapythonista as well for #26887

WillAyd

Can you also add a whatsnew for v1.0.0?

datapythonista · 2019-09-17T09:58:00Z

I'm not sure if the clipboard tests are running in the CI, so we need to be very careful here. I fixed it in #26949, but it made the tests be much slower, and the PR wasn't finally merged. Not sure if anyone else worked on it, but it may be that tests for the clipboard are not running at all, so we can't merge this without fixing that or proper "manual" testing.

Also, I guess there was a reason to copy the dependency to pandas, but may be things changed and now we can simply have it as a dependency?

krey · 2019-09-17T14:18:23Z

If there's no opposition to changing pyperclip to a dependency, I will close this PR and open a new one.

@WillAyd I don't have access to a Windows machine (re #11720)

WillAyd · 2019-09-17T14:34:37Z

If there's no opposition to changing pyperclip to a dependency

I don't think we should do that. If starting from scratch might make sense but I don't see the churn being worth it. These aren't a huge burden to keep included

datapythonista · 2019-09-17T14:44:31Z

I see the advantage of getting updates automatically, not a risky PR like this for every new version of pyperclip. And I don't see a disadvantage, I don't think a PR to make it a dependency is a big change, is it?

krey · 2019-09-17T14:53:19Z

@datapythonista Agreed.

The change is literally #28471 (comment) (and changing some imports in the tests)

TomAugspurger · 2019-09-17T14:57:20Z

Like Will, I'd prefer to keep vendoring it. I don't think it's worth adding as a dependency.

…

On Tue, Sep 17, 2019 at 9:53 AM krey ***@***.***> wrote: @datapythonista <https://github.com/datapythonista> Agreed. The change is literally #28471 (comment) <#28471 (comment)> (and changing some imports in the tests) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#28471?email_source=notifications&email_token=AAKAOIWQCHZKV74OZ3DZAYDQKDVOTA5CNFSM4IXHUSOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD64ZVOA#issuecomment-532257464>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKAOIQEIXW2M5SFMNJY4A3QKDVOTANCNFSM4IXHUSOA> .

jreback

this is fine to vendor and update to current; we don't want to add actual required dependencies (and making this optional is actually somewhat non-trivial).

pls add a whatsnew note on the issue resolution. do we need a test for this issue?

datapythonista

I think I found out the problem that made tests very slow when trying to restore the clipboard tests (afaik they are not yet working).

Once #28482 is merged, I'll give it a try. Probably worth waiting here until we have the clipboard tests back, to make sure this upgrade is not breaking anything.

WillAyd · 2019-09-19T16:01:20Z

@krey looks like the above fix is in. Can you merge master and address comments around the whatsnew / test?

krey · 2019-09-24T15:03:33Z

Shall we add pyperclip's tests?https://github.com/asweigart/pyperclip/blob/master/tests/test_pyperclip.py

WillAyd · 2019-09-24T15:43:23Z

I think that makes sense to include as part of this

WillAyd · 2019-10-10T02:07:33Z

@krey can you fix merge conflicts and pull in the tests?

WillAyd · 2019-10-22T01:34:26Z

pandas/tests/io/test_pyperclip.py

+random.seed(42)  # Make the "random" tests reproducible.
+
+
+class _TestClipboard(unittest.TestCase):


Hmm we don't use unittest so would need to change these - can you see how much effort it is to convert to pytest idiom?

Trying to balance asking a lot here versus getting this to work; unfortunately our current tests for this are suspect

datapythonista · 2019-10-22T01:42:22Z

Do we want to run pyperclip tests? I understand that we don't want to add pyperclip as a hard dependency, and it's too complex to have it as an optional dependency for now. But I think we just want to have it as a vendored dependency, trusting that they run their own tests, and I don't see which value adds to pandas to run their tests (or include them in the project).

What are we going to do if tests fail in our CI? I don't think we want to touch our copy of their source, do we?

Am I missing something?

WillAyd · 2019-10-22T01:43:55Z

I'm not tied to it. I guess I'm unclear as to how we are currently validating / testing these though. Is this back up and running in CI? Or something we need to do as a pre-cursor?

Just hesitant to blanket vendor these changes without some kind of CI validation

jreback · 2019-10-22T01:51:52Z

we don’t need the tests; straight vendor is enough

datapythonista · 2019-10-22T01:58:43Z

pyperclip itself should be tested. And we should have tests for our functions that use it, so if the new version breaks something in our code, the CI should be failing. So I'm with Jeff, we don't need the tests.

WillAyd · 2019-10-22T02:02:47Z

OK reverted just reverted that for @krey let's see

jreback · 2019-11-06T21:33:36Z

@krey this looks good. can you merge master and I think have a linting issue in the whatsnew. ping on green.

…ase object

krey · 2019-11-07T15:28:03Z

@jreback It's mostly green...

WillAyd · 2019-11-07T15:41:08Z

codecov is sometimes wonky so no worries there; rest lgtm as well

WillAyd · 2019-11-07T15:41:37Z

Thanks @krey

simonjayhawkins · 2019-11-07T18:27:25Z

pandas/io/clipboard/__init__.py

+    if not isinstance(text, acceptedTypes):
+        raise PyperclipException(
+            "only str, int, float, and bool values"
+            "can be copied to the clipboard, not".format(text.__class__.__name__)


mypy error: Not all arguments converted during string formatting [str-format]

jbrockmendel reviewed Sep 16, 2019

View reviewed changes

pandas/io/clipboard/__init__.py Show resolved Hide resolved

krey force-pushed the master branch 3 times, most recently from 971200b to 3acd9df Compare September 17, 2019 00:00

WillAyd added the IO Data IO issues that don't fit into a more specific label label Sep 17, 2019

WillAyd added this to the 1.0 milestone Sep 17, 2019

WillAyd requested changes Sep 17, 2019

View reviewed changes

jreback requested changes Sep 18, 2019

View reviewed changes

datapythonista requested changes Sep 18, 2019

View reviewed changes

krey force-pushed the master branch 2 times, most recently from dec0f6c to 9c519bd Compare September 24, 2019 01:54

krey requested review from datapythonista, jreback and WillAyd September 24, 2019 13:46

krey force-pushed the master branch from e461b27 to 8b0ca66 Compare October 21, 2019 22:00

WillAyd requested changes Oct 22, 2019

View reviewed changes

jreback added the Output-Formatting __repr__ of pandas objects, to_string label Nov 6, 2019

jreback approved these changes Nov 6, 2019

View reviewed changes

krey and others added 8 commits November 6, 2019 20:22

Updated internal pyperclip 1.5.27 -> 1.7.0

33cd2d7

CLN: Removed PY2 references

91af512

STYLE: black, flake8, isort, cleaned up super() and inhertance from b…

da46244

…ase object

DOC: Link to new PyPI instead of legacy

fa39346

Fixed clipboard tests.

268028e

Removed outdated gtk package from code

003e08e

TST: added pyperclip's tests

741dcfc

Reverted test additions

4a51a4e

krey force-pushed the master branch from 9dbdace to 4a51a4e Compare November 7, 2019 01:26

WillAyd approved these changes Nov 7, 2019

View reviewed changes

WillAyd merged commit f590cea into pandas-dev:master Nov 7, 2019

simonjayhawkins reviewed Nov 7, 2019

View reviewed changes

Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019

pyperclip upgrade (pandas-dev#28471)

c7e10d3

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

pyperclip upgrade (pandas-dev#28471)

7e66251

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

pyperclip upgrade (pandas-dev#28471)

885e933

rhshadrach mentioned this pull request Mar 4, 2023

BUG: clipboard does not work on wayland #51744

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pyperclip upgrade #28471

pyperclip upgrade #28471

krey commented Sep 16, 2019 •

edited

Loading

jbrockmendel commented Sep 16, 2019

krey commented Sep 16, 2019 •

edited

Loading

jbrockmendel commented Sep 16, 2019

WillAyd commented Sep 17, 2019

krey commented Sep 17, 2019

krey commented Sep 17, 2019

WillAyd commented Sep 17, 2019

WillAyd left a comment

datapythonista commented Sep 17, 2019

krey commented Sep 17, 2019

WillAyd commented Sep 17, 2019

datapythonista commented Sep 17, 2019

krey commented Sep 17, 2019

TomAugspurger commented Sep 17, 2019 via email

jreback left a comment

datapythonista left a comment

WillAyd commented Sep 19, 2019

krey commented Sep 24, 2019

WillAyd commented Sep 24, 2019

WillAyd commented Oct 10, 2019

WillAyd Oct 22, 2019

datapythonista commented Oct 22, 2019

WillAyd commented Oct 22, 2019

jreback commented Oct 22, 2019

datapythonista commented Oct 22, 2019

WillAyd commented Oct 22, 2019

jreback commented Nov 6, 2019

krey commented Nov 7, 2019

WillAyd commented Nov 7, 2019

WillAyd commented Nov 7, 2019

simonjayhawkins Nov 7, 2019

		random.seed(42) # Make the "random" tests reproducible.


		class _TestClipboard(unittest.TestCase):

pyperclip upgrade #28471

pyperclip upgrade #28471

Conversation

krey commented Sep 16, 2019 • edited Loading

jbrockmendel commented Sep 16, 2019

krey commented Sep 16, 2019 • edited Loading

jbrockmendel commented Sep 16, 2019

WillAyd commented Sep 17, 2019

krey commented Sep 17, 2019

krey commented Sep 17, 2019

WillAyd commented Sep 17, 2019

WillAyd left a comment

Choose a reason for hiding this comment

datapythonista commented Sep 17, 2019

krey commented Sep 17, 2019

WillAyd commented Sep 17, 2019

datapythonista commented Sep 17, 2019

krey commented Sep 17, 2019

TomAugspurger commented Sep 17, 2019 via email

jreback left a comment

Choose a reason for hiding this comment

datapythonista left a comment

Choose a reason for hiding this comment

WillAyd commented Sep 19, 2019

krey commented Sep 24, 2019

WillAyd commented Sep 24, 2019

WillAyd commented Oct 10, 2019

WillAyd Oct 22, 2019

Choose a reason for hiding this comment

datapythonista commented Oct 22, 2019

WillAyd commented Oct 22, 2019

jreback commented Oct 22, 2019

datapythonista commented Oct 22, 2019

WillAyd commented Oct 22, 2019

jreback commented Nov 6, 2019

krey commented Nov 7, 2019

WillAyd commented Nov 7, 2019

WillAyd commented Nov 7, 2019

simonjayhawkins Nov 7, 2019

Choose a reason for hiding this comment

krey commented Sep 16, 2019 •

edited

Loading

krey commented Sep 16, 2019 •

edited

Loading