Skip to content

test: Run the exporters in CI instead of skipping them - #87

Merged
m96-chan merged 1 commit into
mainfrom
ci/export-extra
Sep 6, 2026
Merged

m96-chan merged 1 commit into
mainfrom
ci/export-extra

Conversation

@m96-chan

@m96-chan m96-chan commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Summary

The exporters are a documented feature and CI has never run a line of them. It installs the wheel with no extras, so the tests needing pandas or pyarrow skipped, and the report read 194 passed, 2 skipped without mentioning that two of the four export formats were untested.

Left over from #82.

Changes

  • Install the wheel as <wheel>[export] in test-python, so the extra's contents stay defined in pyproject.toml alone rather than being repeated in the workflow.
  • Assert afterwards that pandas and pyarrow imported. A skip is the failure mode this removes; it should not be able to come back quietly.
  • Rework test_export.py so the guards mean what they say.
  • Add round trips for parquet and arrow.

Related Issue

Follow-up from #82; no separate issue.

Why the tests needed work first

Every test in the file was wrapped in:

try:
    from pyetwkit.export import to_csv
    ...
except ImportError:
    pytest.skip("pyetwkit.export module not available")

That is wrong twice over. pyetwkit.export always imports — pandas and pyarrow are imported inside the functions that need them — so eight of the ten guards could only ever have fired on a genuine ImportError raised inside the exporters, reporting a broken module as an absent optional dependency. Same shape as #77, where a wrong import name kept 112 tests skipped for months.

Now the presence of each optional dependency is decided once, up front, and only the tests that actually need one are marked. Everything else asserts.

Also worth noting to_csv goes through to_dataframe, so it needs pandas too — which is why the second skip was in TestToCSV rather than somewhere obvious.

Test Plan

Added/Modified Tests

test_export.py goes from 12 tests to 18: JSON/JSONL round trips, a DataFrame content check, a CSV content check, and parquet/arrow round trips. Both parquet and arrow previously had only a callable() check, so installing pyarrow in CI would otherwise have bought nothing.

Test Results

With the extra, in a clean venv from the built wheel:

$ pip install "pyetwkit-3.0.2-cp312-cp312-win_amd64.whl[export]"
$ python -c "import pandas, pyarrow"
pandas 3.0.5
pyarrow 25.0.1

$ pytest tests/test_export.py
18 passed

Without it, in the same venv before the extra was added — the point being that the twelve which do not need pandas now run rather than skipping, and the six that do skip say what is missing:

12 passed, 6 skipped

SKIPPED tests\test_export.py:145: pandas is not installed (pip install pyetwkit[export])
SKIPPED tests\test_export.py:153: pandas is not installed (pip install pyetwkit[export])
SKIPPED tests\test_export.py:170: pandas is not installed (pip install pyetwkit[export])
SKIPPED tests\test_export.py:185: pandas is not installed (pip install pyetwkit[export])
SKIPPED tests\test_export.py:204: pyarrow is not installed (pip install pyetwkit[export])
SKIPPED tests\test_export.py:225: pyarrow is not installed (pip install pyetwkit[export])

Full suite locally:

324 passed
ruff / black   clean

CI should now report zero skips for this file.

Checklist

  • Tests have been added/updated
  • All tests pass
  • Ready for code review
  • Documentation updated (if applicable)
  • CHANGELOG.md updated (if applicable)

Additional Notes

<wheel>[export] was verified against a real wheel rather than assumed; pip and uv both accept extras on a direct wheel path.

This adds pandas and pyarrow to five matrix entries, which is not free. If the download time is unwelcome, the alternative is a single job that installs the extra and runs only tests/test_export.py, leaving the matrix as it is — worth saying so you can choose, though I would rather the exporters were covered on every supported Python than on one.

Independent of this: dashboard and otlp are extras too, and their tests may have the same shape. I have not looked; happy to if you want it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HhPcm483PrDmuQEUSALBFn

The pandas and pyarrow exporters are a documented feature and CI never ran a
line of them. It installed the wheel with no extras, so the tests that needed
those packages skipped, and the report said "194 passed, 2 skipped" without
saying that two of the four export formats were untested.

Install the wheel as `<wheel>[export]`, which takes the extra's contents from
pyproject.toml rather than repeating pandas and pyarrow here, and assert
afterwards that both imported. A skip is the failure mode this is meant to
remove, so it should not be able to come back quietly.

The tests needed work first. Every one of them was wrapped in

    try:
        from pyetwkit.export import to_csv
        ...
    except ImportError:
        pytest.skip("pyetwkit.export module not available")

which is wrong twice over. The module always imports -- pandas and pyarrow are
imported inside the functions that need them -- so eight of the ten guards
could only ever have fired on a genuine ImportError from inside the exporters,
reporting a broken module as an absent optional dependency. That is the same
shape as #77, where a wrong import name skipped 112 tests for months.

Decide once, up front, whether each optional dependency is installed, and mark
only the tests that actually need one. Everything else now asserts. With the
extra installed all eighteen run; without it, twelve still run and the six that
skip say which package is missing and how to get it, rather than claiming the
module is unavailable.

Also added round trips for parquet and arrow. Both had only a `callable()`
check, so installing pyarrow in CI would otherwise have bought nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HhPcm483PrDmuQEUSALBFn
@m96-chan
m96-chan merged commit d587f2d into main Sep 6, 2026
14 checks passed
@m96-chan
m96-chan deleted the ci/export-extra branch September 6, 2026 06:49
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.

1 participant