Patch stash to run with Pythonista 3.4 beta #463

mkb79 · 2023-01-18T13:29:12Z

launch_stash.py will work. But an ignored exception No method found for selector "setAutomaticallyAdjustsContentInsetForKeyboard:" is raised.

pip seams to fail to download files.

Edit:
The issue with pip is an exception raising in wget. <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:997)>

`launch_stash.py` will work. But an ignore exception `No method found for selector "setAutomaticallyAdjustsContentInsetForKeyboard:"` is raised. pip seams to fail to download files.

bennr01 · 2023-01-18T14:39:54Z

Thank you for your bug report and the patch.
I hadn't yet the chance to try your patch out myself, but looking at the changes I think these changes may still need a bit of improvement:

You use certifi to patch wget. certifi is not part of the default python library, but included in pythonista by default. That's ok, but you'll need to add it as a dependency to setup.py so it will automatically be installed in the test environment too.
In your changes to shcommon.py, you've removed PYTHONISTA_VERSION = ... and PYTHONISTA_VERSION_LONG = .... I assume you wanted to move them behind the definition of the _properties() function and forgot to re-add these defintions. Either way, unless I am missing something here, this may break some stuff. Please re-add those definitions.

mkb79 · 2023-01-18T14:50:49Z

@bennr01

In your changes to shcommon.py, you've removed PYTHONISTA_VERSION = ... and PYTHONISTA_VERSION_LONG = ....

Sorry, my mistake. I've re-added them now.

You use certifi to patch wget.
I can add them to the setup.py or I can add a try/except import to add the cafile only if certifi is installed. What’s the best option your you?

jsbain · 2023-01-19T08:00:03Z

Looks like we need to change isAlive to is_alive in shthreads.py

also, is it just me, or do you have to call _stash.launch() from a background thread? I’m getting a crash on iPadOS 15 otherwise.

Using `Thread.is_alive` in Python 3.9+ and fallback to `Thread.isAlive` to keep backward compatibility.

@jsbain

Using `SUITextView` by default and switch to `SUITextView_PY3` if available. Thanks goes to @jsbain for finding out the solution.

mkb79 · 2023-01-19T08:47:04Z

Looks like we need to change isAlive to is_alive in shthreads.py

Thank you for your information. I've added these to the PR. And thanks for your hint with the SUITextView_PY3.

also, is it just me, or do you have to call _stash.launch() from a background thread? I’m getting a crash on iPadOS 15 otherwise.

Which line of code you mean exactly?

yaroslavyaroslav · 2023-01-20T15:34:54Z

Come on guys, millions of eyes are watching you right now like never before. If OMZ could do it, so can you.

bennr01 · 2023-01-20T15:59:17Z

I can add them to the setup.py or I can add a try/except import to add the cafile only if certifi is installed. What’s the best option your you?

I think adding them to setup.py would be the best, as it would mean less difference between the test environment and user enviroments. Either way, the changes seem to be good now, I'll merge this PR as soon as certifi is added to setup.py.

Come on guys, millions of eyes are watching you right now like never before. If OMZ could do it, so can you.

I think millions of eyes may be a bit optimistic... unless you have like a lot of them in a jar somewhere...

mkb79 · 2023-01-20T16:29:47Z

@bennr01

I think adding them to setup.py would be the best, as it would mean less difference between the test environment and user enviroments. Either way, the changes seem to be good now, I'll merge this PR as soon as certifi is added to setup.py.

certifi is Python 3+ only. So this may break something?!

fschaeck · 2023-01-21T08:47:23Z

certifi is Python 3+ only. So this may break something?!

Since Pythonista 3.4 won‘t support Python 2 anymore and comes with Python 3.10 only, I think it is high time for stash to say Good-Bye to Python 2 as well and make the next release Python 3.10 only. Everybody having scripts that need earlier Python versions will have to stay on Pythonista 3.3 anyway or migrate them to 3.10.

… Coming to think of it, stash might be used in other places than Pythonista as well. Thus it might not be that simple to drop at least Python 2 support.

jsbain · 2023-01-23T05:01:33Z

We could just fork and rename to stash3.

On my iPad 6th gen running ipados15.1 I was getting a crash when running launch_stash. I chenged

_stash.launch(ns.command)

To something wrapping in_background

import ui
@ui.in_background()
def launch(*args):
   _stash.launch(*args)

launch(ns.command)

bennr01 · 2023-01-30T02:13:22Z

Sorry for the delay.

certifi is Python 3+ only. So this may break something?!

That's a good point, I hadn't checked.

I agree with the others that it may finally be time to drop py2, as much as it hurts (py27 FTW!). I propose the following course of action:

designate a clear stash version which starts dropping py2 compatibility
keep getstash.py both py27 and py3 compatible. As this file gets executed during an update, we should modify it to check for the python version and warn for incompatibility and aborting an update when a py2 user attempts to update to an incompatible version of stash
update docs, keeping an info on how to install the last py2-compatible version
focus future changes on py3, but let's don't get out of our way to remove compatibility code unless its a component anyone is actively working on.

Anyone here opposed to this proposal?

As for the PR, I'll merge it and #465 once we've decided on the future of py2 stash. It all looks ok, I don't think any more changes are necessary. Once again, thank you for your contribution.

ikiziki · 2023-02-27T19:36:59Z

I see it has been a month since there was any further activity on this PR, is there an ETA on when it can be merged in to work with Pythonista 3.4 Beta?

cclauss · 2023-03-06T07:18:52Z

.github/workflows/check-code-and-run-tests.yml

+        flake8 . --count --exit-zero --max-complexity=10 --max-line-length=88 --statistics
+    - name: Running tests
+      run: |
+        py.test --version


It is deprecated to run pytest with the dot.

Suggested change

py.test --version

pytest --version

cclauss · 2023-03-06T07:47:45Z

system/shui/pythonista_ui.py

+            # `Thread.isAlive()` was removed in Python 3.9
+            is_render_thread_alive = None
+            if self.render_thread is not None:
+                if hasattr(self.render_thread, 'is_alive'):
+                    is_render_thread_alive = self.render_thread.is_alive()
+                else:
+                    is_render_thread_alive = self.render_thread.isAlive()
+
+            if self.render_thread is None or not is_render_thread_alive:


Thread.is_alive() was added in Python 2.6 so we can merely change .isAlive() to .is_alive() everywhere. See #478

https://docs.python.org/2/library/threading.html#threading.Thread.is_alive

Suggested change

# `Thread.isAlive()` was removed in Python 3.9

is_render_thread_alive = None

if self.render_thread is not None:

if hasattr(self.render_thread, 'is_alive'):

is_render_thread_alive = self.render_thread.is_alive()

else:

is_render_thread_alive = self.render_thread.isAlive()

if self.render_thread is None or not is_render_thread_alive:

if self.render_thread is None or not self.render_thread.is_alive():

cclauss · 2023-03-06T07:50:03Z

system/shthreads.py

+
+        # `Thread.isAlive()` was removed in Python 3.9
+        if hasattr(self, 'is_alive'):
+            is_alive = self.is_alive()
+        else:
+            is_alive = self.isAlive()
+
+        if is_alive:


Thread.is_alive() was added in Python 2.6 so we can merely change .isAlive() to .is_alive() everywhere. See #478

https://docs.python.org/2/library/threading.html#threading.Thread.is_alive

Suggested change

# `Thread.isAlive()` was removed in Python 3.9

if hasattr(self, 'is_alive'):

is_alive = self.is_alive()

else:

is_alive = self.isAlive()

if is_alive:

if self.is_alive():

cclauss · 2023-03-15T07:01:50Z

Please rebase and resolve git conflicts.

bennr01 · 2023-03-15T07:32:26Z

Thank you for your conttribution and you patience. I'll merge it now (Edit: once the tests finish.).

Regarding merge conflicts: No need to rebase and resolve this, this is trivial enough that I can do this quickly myself. (Edit 2 oops, I've messed it up, give me a minute...)

system/shthreads.py

@cclauss

Thanks @cclauss Co-authored-by: Christian Clauss <cclauss@me.com>

.github/workflows/check-code-and-run-tests.yml

cclauss · 2023-03-15T07:39:44Z

Let's add the test run on Python 3.10 because that is what the beta runs.

cclauss · 2023-03-15T07:43:44Z

On legacy Python:

E   Querying PyPI ... 
E   Using rsa==3.2.2...
E   A binary distribution is available and will be used.

Can we pip install --no-binary?
Or PIP_NO_BINARY=rsa pipenv install ...?

bennr01 · 2023-03-15T07:55:59Z

On legacy Python:
E   Querying PyPI ... 
E   Using rsa==3.2.2...
E   A binary distribution is available and will be used.
Can we pip install --no-binary?

Yes, we could, but shouldn't we rather figure out why the binary install is failing? Basic wheel support is included and worked rather well so far (which is why our pip implementation prefers it over our setuptools mock). Either way, this seems like its not caused by the PR itself but rather due to the merge with the dev-branch, so I'd merge it even with that specific test failure. I am more worried that the py39 tests took 16 minutes...

cclauss · 2023-03-15T08:02:53Z

I don't think that Apple's security guidelines for iOS are going to allow a binary install.

Please add py310 (see above) and run the tests again. That long run might have been a fluke.

Patch stash to run with Pythonista 3.4 beta #463 (comment)

See discussion in ywangd#463 Co-authored-by: Christian Clauss <cclauss@me.com>

bennr01 · 2023-03-15T08:14:05Z

I don't think that Apple's security guidelines for iOS are going to allow a binary install.

In this case "Binary" does not refer to a compiled file, but rather a distribution that does not require the execution of an installer. It's been a while since I worked on pip, but IIRC nearly all source distribution require arbitrary python code as part of the setup.py file (which may invoke a C-compilation), while binary distributions (like wheels) may include compiled extensions (but we don't support this) or may not include compiled extensions (as indicated by the -none-any part of the wheel name), but more importantly change the setuptools.setup() tool into a couple of static configuration/metadata files.

Anyway, you were right that the py36 failure was a fluke, I'll go ahead and finally merge this PR.

fschaeck and others added 6 commits June 6, 2022 14:58

Creating git workflow to replace .travis.yml

2615d90

Only test for Python 2.7, 3.6 and 3.8 for now

11a25af

Fixing dependency versions for GITHUB CI/CD action

ad86d45

Attempt to fix tests

9aff137

Fixing pyte version and missing PYTHONPATH for install test

b722d71

Patch stash to run with Pythonista 3.4 beta

cb96166

`launch_stash.py` will work. But an ignore exception `No method found for selector "setAutomaticallyAdjustsContentInsetForKeyboard:"` is raised. pip seams to fail to download files.

mkb79 mentioned this pull request Jan 18, 2023

stash does not run with Pythonista v3.4 Beta #462

Open

Patch ssl issue in wget and Pythonista 3.4

73ed7f3

re-add PYTHONISTA_VERSION and PYTHONISTA_VERSION_LONG

b3b17e5

mkb79 added 3 commits January 19, 2023 09:35

Fix: Thread.isAlive() was removed in Python 3.9

e160367

Using `Thread.is_alive` in Python 3.9+ and fallback to `Thread.isAlive` to keep backward compatibility.

Fix SUITextView issue

ff0b99a

Using `SUITextView` by default and switch to `SUITextView_PY3` if available. Thanks goes to @jsbain for finding out the solution.

correct docstrings and comments for SUITextView

b002219

mkb79 changed the base branch from master to dev January 20, 2023 16:41

cclauss reviewed Mar 6, 2023

View reviewed changes

This was referenced Mar 6, 2023

Upgrade to plistlib.load(in_file) #472

Closed

Replace Thread.isAlive() with Thread.is_alive() #473

Closed

cclauss reviewed Mar 6, 2023

View reviewed changes

cclauss mentioned this pull request Mar 6, 2023

Replace Thread.isAlive() with Thread.is_alive() AGAIN #478

Merged

cclauss mentioned this pull request Mar 6, 2023

Add GitHub Actions on dev branch #477

Merged

Merge branch 'dev' into patch-Pythonista3.4

f882779

cclauss reviewed Mar 15, 2023

View reviewed changes

system/shthreads.py Outdated Show resolved Hide resolved

Fix wrong merge conflic resolution (NameError)

8d9e5a3

Thanks @cclauss Co-authored-by: Christian Clauss <cclauss@me.com>

cclauss reviewed Mar 15, 2023

View reviewed changes

.github/workflows/check-code-and-run-tests.yml Outdated Show resolved Hide resolved

Add py 3.10 to test matrix

9c5fac1

See discussion in ywangd#463 Co-authored-by: Christian Clauss <cclauss@me.com>

bennr01 merged commit 2549d26 into ywangd:dev Mar 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Patch stash to run with Pythonista 3.4 beta #463

Patch stash to run with Pythonista 3.4 beta #463

mkb79 commented Jan 18, 2023 •

edited

Loading

bennr01 commented Jan 18, 2023

mkb79 commented Jan 18, 2023

jsbain commented Jan 19, 2023

mkb79 commented Jan 19, 2023 •

edited

Loading

yaroslavyaroslav commented Jan 20, 2023

bennr01 commented Jan 20, 2023

mkb79 commented Jan 20, 2023

fschaeck commented Jan 21, 2023 •

edited

Loading

jsbain commented Jan 23, 2023

bennr01 commented Jan 30, 2023

ikiziki commented Feb 27, 2023

cclauss Mar 6, 2023 •

edited

Loading

cclauss Mar 6, 2023 •

edited

Loading

cclauss Mar 6, 2023 •

edited

Loading

cclauss commented Mar 15, 2023

bennr01 commented Mar 15, 2023 •

edited

Loading

cclauss commented Mar 15, 2023 •

edited

Loading

cclauss commented Mar 15, 2023 •

edited

Loading

bennr01 commented Mar 15, 2023

cclauss commented Mar 15, 2023 •

edited

Loading

bennr01 commented Mar 15, 2023

Patch stash to run with Pythonista 3.4 beta #463

Patch stash to run with Pythonista 3.4 beta #463

Conversation

mkb79 commented Jan 18, 2023 • edited Loading

bennr01 commented Jan 18, 2023

mkb79 commented Jan 18, 2023

jsbain commented Jan 19, 2023

mkb79 commented Jan 19, 2023 • edited Loading

yaroslavyaroslav commented Jan 20, 2023

bennr01 commented Jan 20, 2023

mkb79 commented Jan 20, 2023

fschaeck commented Jan 21, 2023 • edited Loading

jsbain commented Jan 23, 2023

bennr01 commented Jan 30, 2023

ikiziki commented Feb 27, 2023

cclauss Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

cclauss Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

cclauss Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

cclauss commented Mar 15, 2023

bennr01 commented Mar 15, 2023 • edited Loading

cclauss commented Mar 15, 2023 • edited Loading

cclauss commented Mar 15, 2023 • edited Loading

bennr01 commented Mar 15, 2023

cclauss commented Mar 15, 2023 • edited Loading

bennr01 commented Mar 15, 2023

mkb79 commented Jan 18, 2023 •

edited

Loading

mkb79 commented Jan 19, 2023 •

edited

Loading

fschaeck commented Jan 21, 2023 •

edited

Loading

cclauss Mar 6, 2023 •

edited

Loading

cclauss Mar 6, 2023 •

edited

Loading

cclauss Mar 6, 2023 •

edited

Loading

bennr01 commented Mar 15, 2023 •

edited

Loading

cclauss commented Mar 15, 2023 •

edited

Loading

cclauss commented Mar 15, 2023 •

edited

Loading

cclauss commented Mar 15, 2023 •

edited

Loading