Skip to content

Conversation

@urasakikeisuke
Copy link
Collaborator

Summary

This PR refines the dependency logic introduced in v1.4.1.
It restricts the fallback to python-lzf to Linux ARM64 environments only, restoring python-neo-lzf for macOS ARM64 (Apple Silicon) users.

Motivation

As pointed out in this comment by @johnthagen, python-neo-lzf provides Universal2 wheels for macOS, which support ARM64.
Therefore, forcing macOS users to use python-lzf was unnecessary. The build failure is specific to Linux aarch64 (e.g., Docker containers, Raspberry Pi) where wheels are missing.

Changes

Modified pyproject.toml to check sys_platform in addition to platform_machine.

  • Before (v1.4.1):
    • python-lzf on ALL arm64/aarch64
  • After (This PR):
    • python-lzf on Linux AND (arm64/aarch64)
    • python-neo-lzf on everything else (including macOS ARM64)

Verification

We have verified the installation behavior on the following physical and emulated environments:

Environment Architecture Type Result
Linux (Ubuntu) x86_64 Native (Physical) Installed python-neo-lzf
macOS Apple Silicon (M1/M2) Native (Physical) Installed python-neo-lzf ✅ (Fixed)
Linux (Docker) aarch64 Emulator (QEMU) Installed python-lzf ✅ (Fallback)

Future Plan

We are tracking the upstream issue: FledgeXu/python-neo-lzf#2.
Once Linux aarch64 wheels are provided by python-neo-lzf, we will revert this fallback entirely.

In v1.4.1, the dependency was switched to `python-lzf` for all ARM64 platforms to resolve installation issues.
However, `python-neo-lzf` already provides Universal2 wheels for macOS (supporting Apple Silicon), so the fallback is strictly necessary only for Linux aarch64 environments where pre-built wheels are missing.

This commit refines the PEP 508 environment markers to:
- Use `python-lzf` ONLY on Linux aarch64/arm64.
- Restore `python-neo-lzf` for macOS ARM64 (and continue using it for x86_64).
@urasakikeisuke urasakikeisuke self-assigned this Dec 12, 2025
Copy link

@windsurf-bot windsurf-bot bot left a comment

Choose a reason for hiding this comment

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

Looks good to me 🤙

💡 To request another review, post a new comment with "/windsurf-review".

@urasakikeisuke urasakikeisuke added rd/fix Bug fix for the user, not a fix to a build script rd/patch labels Dec 12, 2025
@github-actions
Copy link

github-actions bot commented Dec 12, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
src/pypcd4
   __init__.py20100% 
   _version.py18478%15–19
   pointcloud2.py591280%87–101
   pypcd4.py4244889%24–25, 136, 189, 352–353, 567, 583–605, 620–626, 636–646, 657–697, 707–708, 927–929, 949–950, 1071, 1076, 1079
TOTAL5036487% 

Tests Skipped Failures Errors Time
67 1 💤 0 ❌ 0 🔥 0.828s ⏱️

@urasakikeisuke urasakikeisuke merged commit d8786f2 into main Dec 14, 2025
12 checks passed
@urasakikeisuke urasakikeisuke deleted the fix/refine-arm-dependency branch December 14, 2025 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rd/fix Bug fix for the user, not a fix to a build script rd/patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants