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.

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

Support for pyodide 0.27.1, including pyarrow + pandas #2901

Merged
merged 4 commits into from
Feb 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions .github/actions/install-deps/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -197,21 +197,6 @@ runs:
fi
if: ${{ runner.os == 'Linux' && inputs.cpp == 'true' && inputs.javascript == 'false' }}

- name: Install Python Pyodide dependencies
shell: bash
run: python -m pip install -r rust/perspective-python/requirements-pyodide.txt
if: ${{ inputs.pyodide == 'true' }}

- name: Install Pyodide distribution
shell: bash
run: pnpm run install_pyodide
if: ${{ inputs.pyodide == 'true' }}

- name: Install Python Playwright browsers
shell: bash
run: python -m playwright install
if: ${{ inputs.pyodide == 'true' }}

Copy link
Member

Choose a reason for hiding this comment

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

Why did we move these?

# - name: Install CCache
# shell: bash
# run: sudo apt install -y ccache
Expand Down
60 changes: 53 additions & 7 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,6 @@ jobs:
id: init-step
uses: ./.github/actions/install-deps
with:
pyodide: "true"
javascript: "false"
arch: ${{ matrix.arch }}
manylinux: "false"
Expand All @@ -411,18 +410,64 @@ jobs:
PACKAGE: "perspective-python"
CI: 1

- uses: actions/upload-artifact@v4
with:
name: perspective-python-dist-wasm32-emscripten-${{ matrix.python-version }}
path: rust/target/wheels/*.whl

# ~*~ Test pyodide ~*~
test_emscripten_wheel:
runs-on: ${{ matrix.os }}
needs: [build_emscripten_wheel]
strategy:
fail-fast: false
matrix:
os:
- ubuntu-22.04
arch:
- x86_64
python-version:
- 3.9
node-version: [20.x]
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Config
id: config-step
uses: ./.github/actions/config

- name: Initialize Build
uses: ./.github/actions/install-deps
with:
javascript: "false"
arch: ${{ matrix.arch }}
manylinux: "false"
skip_cache: ${{ steps.config-step.outputs.SKIP_CACHE }}

- uses: actions/download-artifact@v4
with:
name: perspective-python-dist-wasm32-emscripten-${{ matrix.python-version }}
path: rust/target/wheels/

- name: Install Python Pyodide dependencies
shell: bash
run: python -m pip install -r rust/perspective-python/requirements-pyodide.txt

- name: Install Pyodide distribution
shell: bash
run: pnpm run install_pyodide

- name: Install Python Playwright browsers
shell: bash
run: python -m playwright install

- name: "Test Pyodide"
run: pnpm run test
env:
PSP_PYODIDE: 1
PACKAGE: "perspective-python"
CI: 1

- uses: actions/upload-artifact@v4
with:
name: perspective-python-dist-wasm32-emscripten-${{ matrix.python-version }}
path: rust/target/wheels/*.whl

# ,-,---. . . . ,--,--' .
# '|___/ . . . | ,-| ,-. ,-. ,-| `- | ,-. ,-. |-
# ,| \ | | | | | | ,-| | | | | , | |-' `-. |
Expand Down Expand Up @@ -859,6 +904,7 @@ jobs:
benchmark_js,
benchmark_python,
build_emscripten_wheel,
test_emscripten_wheel,
build_and_test_rust,
lint_and_docs,
]
Expand Down
1 change: 1 addition & 0 deletions cmake/modules/FindInstallDependency.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ function(psp_build_dep name cmake_file)
message(FATAL_ERROR "Build step for ${name} failed: ${result}")
endif()
endif()

if(${name} STREQUAL arrow)
set(ARROW_DEFINE_OPTIONS ON)
set(ARROW_BUILD_SHARED OFF)
Expand Down
15 changes: 9 additions & 6 deletions cpp/perspective/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,15 @@ if(PSP_PYODIDE)
string(APPEND CMAKE_CXX_FLAGS "${RELOCATABLE_FLAGS}")
endif()

if(PSP_PYODIDE)
Copy link
Member

Choose a reason for hiding this comment

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

I propose we set this for everything.

string(APPEND CMAKE_CXX_FLAGS " -fvisibility=hidden")
endif()

if (PSP_PYODIDE AND NOT PSP_WASM_EXCEPTIONS)
# Emscripten exceptions
string(APPEND CMAKE_CXX_FLAGS " -fexceptions")
endif()

# Build (read: download and extract) header-only dependencies from external sources
set(all_deps_INCLUDE_DIRS "")
psp_build_dep("date" "${PSP_CMAKE_MODULE_PATH}/date.txt.in")
Expand Down Expand Up @@ -608,11 +617,6 @@ elseif(PSP_CPP_BUILD OR PSP_PYTHON_BUILD)
target_include_directories(psp PRIVATE ${psp_INCLUDE_DIRS})
target_include_directories(psp SYSTEM PRIVATE ${all_deps_INCLUDE_DIRS})
target_compile_definitions(psp PRIVATE PSP_ENABLE_PYTHON=1 PSP_ENABLE_WASM=1)
# support for emscripten exceptions https://emscripten.org/docs/porting/exceptions.html#emscripten-javascript-based-exception-support
target_compile_options(psp PUBLIC -fexceptions -fvisibility=hidden)
target_compile_options(arrow_static PUBLIC -fexceptions -fvisibility=hidden)
target_compile_options(re2 PUBLIC -fexceptions -fvisibility=hidden)
target_compile_options(protos PUBLIC -fexceptions -fvisibility=hidden)
else()
# Cpython
add_library(psp STATIC ${PYTHON_SOURCE_FILES})
Expand All @@ -631,7 +635,6 @@ elseif(PSP_CPP_BUILD OR PSP_PYTHON_BUILD)
target_compile_options(psp PRIVATE -fvisibility=hidden)
endif()

# Link against arrow static library
# Linking against arrow_static also links against its bundled dependencies
target_link_libraries(psp PRIVATE arrow_static re2 protos)
else()
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"type": "module",
"emscripten": "3.1.58",
"llvm": "17.0.6",
"pyodide": "0.26.2",
"pyodide": "0.27.1",
"engines": {
"node": ">=16"
},
Expand Down
34 changes: 33 additions & 1 deletion rust/perspective-python/pyodide-tests/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,37 @@
# pyodide-tests

Smoke and integration tests for the perspective-python Pyodide wheel.
Smoke and integration tests for the perspective-python Pyodide wheel. The tests
are specced in `pytest` and executed with `playwright`, using the
`pytest-pyodide` package.

These tests require that a Pyodide wheel has been built to rust/target/wheels

## test setup

Create a virtual environment. Install perspective-python requirements and
special pyodide-only requirements:

```
pip install -r rust/perspective-python/requirements.txt
pip install -r rust/perspective-python/requirements-pyodide.txt
```

## running tests

Run setup, select `perspective-pyodide` target:

```
pnpm -w run setup
```

Then run tests:

```
pnpm -w test
```

If you are prompted to install playwright browsers, run this in your venv:

```
python -m playwright install
```
43 changes: 43 additions & 0 deletions rust/perspective-python/pyodide-tests/tests/test_smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ def psp_wheel_path(pytestconfig):

# Based on micropip's test server fixture:
# https://github.com/pyodide/micropip/blob/eb8c4497d742e515d24d532db2b9cc014328265b/tests/conftest.py#L64-L87
# Run web server serving the directory containing the perspective wheel,
# yielding the wheel's URL for the fixture value
@pytest.fixture()
def psp_wheel_url(psp_wheel_path):
from pathlib import Path
Expand Down Expand Up @@ -72,3 +74,44 @@ async def test_parsing_good_csv(selenium):
client = server.new_local_client()
abc123 = client.table("a,b,c\n1,2,3\n")
assert abc123.columns() == ["a", "b", "c"]


# Test that perspective can load pyarrow Table values
@run_in_pyodide
async def test_perspective_and_pyarrow_together_at_last(selenium):
import micropip

await micropip.install("pyarrow")

import perspective
import pyarrow as pa

n_legs = pa.array([2, 2, 4, 4, 5, 100])
animals = pa.array(
["Flamingo", "Parrot", "Dog", "Horse", "Brittle stars", "Centipede"]
)
names = ["n_legs", "animals"]
table = pa.Table.from_arrays([n_legs, animals], names=names)

server = perspective.Server()
client = server.new_local_client()

animals = client.table(table, name="animals")
assert animals.columns() == ["n_legs", "animals"]


# Test that perspective can load pandas DataFrame values
@run_in_pyodide
async def test_pandas_dataframes(selenium):
import micropip

await micropip.install(["pyarrow", "pandas"])

import perspective
import pandas as pd

server = perspective.Server()
client = server.new_local_client()
df = pd.DataFrame({"a": [1, 2, 3], "b": [4, 5, 6]})
ab = client.table(df, name="ab")
assert sorted(ab.columns()) == ["a", "b", "index"]
2 changes: 1 addition & 1 deletion rust/perspective-python/requirements-pyodide.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
pytest==8.2.2
pytest-playwright==0.5.2
pytest-pyodide==0.58.3

pytest-timeout==2.3.1
6 changes: 6 additions & 0 deletions rust/perspective-python/test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ if (process.env.PSP_PYODIDE) {
"--runtime=chrome",
`--dist-dir=${pyodideDistDir}`,
`--perspective-emscripten-wheel=${emscriptenWheel}`,
"--verbose",
"--timeout=300",
// with --timeout_method=signal, tests hang. seems like the
// pytest-pyodide webserver fixture does not shut down
"--timeout_method=thread",
...process.argv.slice(2),
],
execOpts
);
Expand Down
9 changes: 8 additions & 1 deletion tools/perspective-scripts/setup.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,14 @@ async function focus_package() {
message: "Focus NPM package(s)?",
default: () => {
if (CONFIG["PACKAGE"]) {
return CONFIG["PACKAGE"].split(",");
const packages = CONFIG["PACKAGE"].split(",");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to test this, runpnpm -w run setup, select perspective-pyodide. then run setup again. perspective-pyodide should be still selected in the menu.

previously the menu selected perspective-python on the second go - which made it easy to accidentally switch back to cpython

if (CONFIG["PSP_PYODIDE"] === "1") {
const py = packages.indexOf("perspective-python");
if (py >= 0) {
packages[py] = "perspective-pyodide";
}
}
return packages;
} else {
return [""];
}
Expand Down
Loading