Skip to content

refactor(lib): remove duplicated library-search blocks#113

Merged
cboulay merged 1 commit into
labstreaminglayer:mainfrom
sappelhoff:perf/dedupe-lib-search
Jun 16, 2026
Merged

refactor(lib): remove duplicated library-search blocks#113
cboulay merged 1 commit into
labstreaminglayer:mainfrom
sappelhoff:perf/dedupe-lib-search

Conversation

@sappelhoff

Copy link
Copy Markdown
Contributor

(PR and content generated with the help of Claude)

What

Remove duplicated code in find_liblsl_libraries (src/pylsl/lib/__init__.py).

Why

The "active Python env lib dir" block and the "macOS Frameworks" block were each present twice, verbatim. Because the generator is consumed with next() (the loader takes the first yielded path), the second copy could only ever re-yield a path already produced by the first — it was effectively dead code.

Impact

  • Removes 24 lines of duplicate code.
  • Eliminates up to 8 redundant os.path.isfile() calls in the fallback path (when the library hasn't been found by an earlier search location). This only runs at import time, so the runtime impact is negligible — this is primarily a correctness/clarity cleanup.

Behavior

Unchanged: the set and order of yielded candidate paths is identical (the duplicate added no new paths). import pylsl and find_liblsl_libraries() verified working after the change; existing test suite passes.

The 'active Python env lib dir' and 'macOS Frameworks' search blocks in
find_liblsl_libraries were each present twice, verbatim. The generator
is consumed with next(), so the second copy could only ever yield a path
already yielded by the first - it was dead code. Removing it drops up to
8 redundant os.path.isfile() calls on the library-not-found fallback path
and leaves the set and order of yielded paths unchanged.
Comment thread src/pylsl/lib/__init__.py
if os.path.isfile(path):
yield path

# The active Python env's lib directory (e.g. `conda install -c conda-forge

@sappelhoff sappelhoff Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

when reviewing this chunk and wondering why it is entirely removed ... expand the lines upward and you see the exact same block repeated.

@cboulay cboulay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Super weird how that happened in the first place. Thanks for taking care of it.

@cboulay cboulay merged commit 37e6122 into labstreaminglayer:main Jun 16, 2026
19 checks passed
@sappelhoff sappelhoff deleted the perf/dedupe-lib-search branch June 16, 2026 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants