Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add hook for pylsl #573

Merged
merged 13 commits into from
May 4, 2023
Merged

Add hook for pylsl #573

merged 13 commits into from
May 4, 2023

Conversation

robertoostenveld
Copy link
Contributor

This adds a hook for pylsl, i.e., the Python wrapper around labstreaminglayer, which is also known as LSL.

The pylsl package loads the C++ liblsl library using CDLL. The logic for locating liblsl is replicated here; subsequently it is added to the binaries and packaged in the compiled version in pylsl/lib which ensures that pylsl will be able to find it.

See also

I tested it successfully on macOS and Windows, using a few methods to install liblsl (using conda, using homebrew, and using a local copy and the PYLSL_LIB variable).

@robertoostenveld robertoostenveld marked this pull request as ready for review April 23, 2023 11:25
@robertoostenveld
Copy link
Contributor Author

I am not sure how I should implement the test. The reason pylsl needs a special hook is because it loads a library (liblsl.so, liblsl.dylib or lsl.dll). The library is not installed by default. The documentation at https://pypi.org/project/pylsl/

On all non-Windows platforms and for some Windows-Python combinations, you must first obtain a liblsl shared library:
...

This requires either conda, homebrew, a "random" downloaded library, or compiling it yourself from the C++ source code. I don't think that any of these would work with the pyinstaller hooks test script.

bwoodsend
bwoodsend reviewed Apr 23, 2023
View reviewed changes
Copy link
Member

@bwoodsend bwoodsend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires either conda, homebrew, a "random" downloaded library, or compiling it yourself from the C++ source code. I don't think that any of these would work with the pyinstaller hooks test script.

If it's on brew, then installing it here then using ; sys_platform == "darwin" in the requirements-test-libraries.txt file would at least give us coverage on one platform which is a lot better than none at all.

src/_pyinstaller_hooks_contrib/hooks/stdhooks/hook-pylsl.py Outdated Show resolved Hide resolved
@robertoostenveld
point to the specific location for the brew recipe at labstreaminglay…
dafc75b 
…er/tap/lsl
@robertoostenveld
Deal with linter errors
80ce5f6 
- src/_pyinstaller_hooks_contrib/hooks/stdhooks/hook-pylsl.py:14:53: 
W291 trailing whitespace
- src/_pyinstaller_hooks_contrib/hooks/stdhooks/hook-pylsl.py:16:1: E302 
expected 2 blank lines, found 1
- src/_pyinstaller_hooks_contrib/hooks/stdhooks/hook-pylsl.py:20:1: W293 
blank line contains whitespace
- src/_pyinstaller_hooks_contrib/hooks/stdhooks/hook-pylsl.py:25:5: E722 
do not use bare 'except'
- src/_pyinstaller_hooks_contrib/hooks/stdhooks/hook-pylsl.py:30:1: E305 
expected 2 blank lines after class or function definition, found 1
@robertoostenveld
also point to the correct lsl in the hidden files under .github
140e0d0
bwoodsend
bwoodsend requested changes May 2, 2023
View reviewed changes
Copy link
Member

@bwoodsend bwoodsend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we'll need a test for pylsl too then.

@bwoodsend bwoodsend merged commit 748248c into pyinstaller:master May 4, 2023
@bwoodsend
Copy link
Member

Thanks @robertoostenveld

wxx9248 pushed a commit to wxx9248/Pickle-Rush that referenced this pull request May 14, 2023
@dependabot
Dependency Updates: Bump pyinstaller-hooks-contrib from 2023.2 to 202…
bc6613f 
…3.3 (#49)

Bumps
[pyinstaller-hooks-contrib](https://github.com/pyinstaller/pyinstaller-hooks-contrib)
from 2023.2 to 2023.3.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/releases">pyinstaller-hooks-contrib's
releases</a>.</em></p>
<blockquote>
<h2>2023.3</h2>
<p>Please see the <a
href="https://www.github.com/pyinstaller/pyinstaller-hooks-contrib/tree/master/CHANGELOG.rst">changelog</a>
for more details</p>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/blob/master/CHANGELOG.rst">pyinstaller-hooks-contrib's
changelog</a>.</em></p>
<blockquote>
<h2>2023.3 (2023-05-11)</h2>
<p>New hooks</p>
<pre><code>
* Add hook for ``graphql_query``
(`[#579](pyinstaller/pyinstaller-hooks-contrib#579)

&lt;https://github.com/pyinstaller/pyinstaller-hooks-contrib/issues/579&gt;`_)
* Add hook for ``pylsl``
(`[#573](pyinstaller/pyinstaller-hooks-contrib#573)

&lt;https://github.com/pyinstaller/pyinstaller-hooks-contrib/issues/573&gt;`_)
<p>Updated hooks
</code></pre></p>
<ul>
<li>Remove no longer needed <code>py</code> hidden imports for
<code>pyshark &gt;= 0.6</code>.
(<code>[#575](pyinstaller/pyinstaller-hooks-contrib#575)
&lt;https://github.com/pyinstaller/pyinstaller-hooks-contrib/issues/575&gt;</code>_)</li>
<li>Update <code>pydantic</code> hook hidden imports to include the
optional dependency
<code>email_validator</code>.
(<code>[#576](pyinstaller/pyinstaller-hooks-contrib#576)
&lt;https://github.com/pyinstaller/pyinstaller-hooks-contrib/issues/576&gt;</code>_)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/30cf5e2682cfaba648301d2375061a80b937191d"><code>30cf5e2</code></a>
Release v2023.3</li>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/f3fbddbb0723e4581331153131174e052e31e080"><code>f3fbddb</code></a>
Add hook for graphql_query</li>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/e2260498ccce78cd09bbe00782f60b414ab7302b"><code>e226049</code></a>
Scheduled weekly dependency update for week 19 (<a
href="https://redirect.github.com/pyinstaller/pyinstaller-hooks-contrib/issues/578">#578</a>)</li>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/748248c2498634c50417d6ba8ad30928a051687a"><code>748248c</code></a>
Add hook for pylsl (<a
href="https://redirect.github.com/pyinstaller/pyinstaller-hooks-contrib/issues/573">#573</a>)</li>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/3d63cfa5887d53d8c4a362edcc421b4a37257da9"><code>3d63cfa</code></a>
Scheduled weekly dependency update for week 17 (<a
href="https://redirect.github.com/pyinstaller/pyinstaller-hooks-contrib/issues/577">#577</a>)</li>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/e47423d27c0a3774bc482fa8dd8d03c31aa1a9b2"><code>e47423d</code></a>
Add email_validator to the list of pydantic optional dependencies hidden
impo...</li>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/0f5685dc82b7be77de6ae1cd4e847af57a82e1bc"><code>0f5685d</code></a>
Remove py hidden imports for pyshark &gt;= 0.6.</li>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/17afa61f7485960f890572b4df46f7d8e1335f4e"><code>17afa61</code></a>
Scheduled weekly dependency update for week 17 (<a
href="https://redirect.github.com/pyinstaller/pyinstaller-hooks-contrib/issues/574">#574</a>)</li>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/79f28de9dc7662dd011d19ed6f1d1db3ca006afe"><code>79f28de</code></a>
Scheduled weekly dependency update for week 16 (<a
href="https://redirect.github.com/pyinstaller/pyinstaller-hooks-contrib/issues/572">#572</a>)</li>
<li><a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/commit/f18145f4e5249ce042db30a8fcd7eaa622464186"><code>f18145f</code></a>
Scheduled weekly dependency update for week 15 (<a
href="https://redirect.github.com/pyinstaller/pyinstaller-hooks-contrib/issues/571">#571</a>)</li>
<li>See full diff in <a
href="https://github.com/pyinstaller/pyinstaller-hooks-contrib/compare/2023.2...2023.3">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pyinstaller-hooks-contrib&package-manager=pip&previous-version=2023.2&new-version=2023.3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rokm rokm rokm left review comments

@bwoodsend bwoodsend bwoodsend approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@robertoostenveld @bwoodsend @rokm