-
Notifications
You must be signed in to change notification settings - Fork 241
Add CSV and JSON export utilities for PSLab sensor data - #261
Add CSV and JSON export utilities for PSLab sensor data #261saadiqbal09 wants to merge 2 commits into
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds pytest-based tests around new CSV and JSON data export utilities for PSLab sensor data, including file creation checks and validation for empty input data. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
saadiqbal09
commented
Dec 26, 2025
This is my first contribution to PSLab.
Happy to update the implementation based on feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 2 issues, and left some high level feedback:
- Consider defining and enforcing a consistent behavior for empty input across both
export_to_csvandexport_to_json(e.g., both raisingValueErroror both producing empty files), so callers don’t have to handle each exporter differently. - It may be useful for
export_to_csvandexport_to_jsonto accept file-like objects in addition to file paths, which would make them easier to integrate with in-memory streams and higher-level APIs. - Clarify and standardize how the exporters handle records with heterogeneous or missing keys (e.g., column ordering, default values), and ensure the implementation consistently applies that strategy.
Prompt for AI Agents
Please address the comments from this code review: ## Overall Comments - Consider defining and enforcing a consistent behavior for empty input across both `export_to_csv` and `export_to_json` (e.g., both raising `ValueError` or both producing empty files), so callers don’t have to handle each exporter differently. - It may be useful for `export_to_csv` and `export_to_json` to accept file-like objects in addition to file paths, which would make them easier to integrate with in-memory streams and higher-level APIs. - Clarify and standardize how the exporters handle records with heterogeneous or missing keys (e.g., column ordering, default values), and ensure the implementation consistently applies that strategy. ## Individual Comments ### Comment 1 <location> `tests/test_data_export.py:4-8` </location> <code_context> +import pytest +from pslab.utils.data_export import export_to_csv, export_to_json + +def test_export_csv_creates_file(tmp_path): + data = [{"a": 1, "b": 2}] + file_path = tmp_path / "data.csv" + export_to_csv(data, str(file_path)) + assert file_path.exists() + +def test_export_json_creates_file(tmp_path): </code_context> <issue_to_address> **suggestion (testing):** CSV export test only checks that the file exists but not that its contents are correct. This test would still pass if the CSV had wrong headers, delimiters, or malformed rows. Please extend it to read and assert the contents—for example, that the header matches the expected columns, the row count is correct, and at least one data row matches the input—so it verifies the export logic, not just file creation. Suggested implementation: ```python import csv import pytest from pslab.utils.data_export import export_to_csv, export_to_json def test_export_csv_creates_file(tmp_path): data = [{"a": 1, "b": 2}] file_path = tmp_path / "data.csv" export_to_csv(data, str(file_path)) # File is created assert file_path.exists() # Contents are correct with file_path.open(newline="") as f: reader = csv.DictReader(f) # Header matches expected columns assert reader.fieldnames == ["a", "b"] rows = list(reader) # Row count is correct assert len(rows) == 1 row = rows[0] # Row data matches input (compare as strings to be robust) assert row["a"] == str(data[0]["a"]) assert row["b"] == str(data[0]["b"]) ``` ```python def test_export_json_creates_file(tmp_path): data = [{"x": 10}] file_path = tmp_path / "data.json" export_to_json(data, str(file_path)) assert file_path.exists() ``` </issue_to_address> ### Comment 2 <location> `tests/test_data_export.py:10-8` </location> <code_context> + export_to_csv(data, str(file_path)) + assert file_path.exists() + +def test_export_json_creates_file(tmp_path): + data = [{"x": 10}] + file_path = tmp_path / "data.json" + export_to_json(data, str(file_path)) + assert file_path.exists() + +def test_export_empty_data_raises_error(): </code_context> <issue_to_address> **suggestion (testing):** JSON export test should validate the serialized content, not just file creation. Consider mirroring the CSV test by loading the JSON (e.g., with `json.load`) and asserting the parsed structure and values match `data`. This will catch issues with encoding, formatting, or changes to the top-level container type. Suggested implementation: ```python import json import pytest from pslab.utils.data_export import export_to_csv, export_to_json ``` ```python def test_export_json_creates_file(tmp_path): data = [{"x": 10}] file_path = tmp_path / "data.json" export_to_json(data, str(file_path)) assert file_path.exists() with file_path.open("r", encoding="utf-8") as f: loaded = json.load(f) assert loaded == data ``` </issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (testing): CSV export test only checks that the file exists but not that its contents are correct.
This test would still pass if the CSV had wrong headers, delimiters, or malformed rows. Please extend it to read and assert the contents—for example, that the header matches the expected columns, the row count is correct, and at least one data row matches the input—so it verifies the export logic, not just file creation.
Suggested implementation:
import csv import pytest from pslab.utils.data_export import export_to_csv, export_to_json def test_export_csv_creates_file(tmp_path): data = [{"a": 1, "b": 2}] file_path = tmp_path / "data.csv" export_to_csv(data, str(file_path)) # File is created assert file_path.exists() # Contents are correct with file_path.open(newline="") as f: reader = csv.DictReader(f) # Header matches expected columns assert reader.fieldnames == ["a", "b"] rows = list(reader) # Row count is correct assert len(rows) == 1 row = rows[0] # Row data matches input (compare as strings to be robust) assert row["a"] == str(data[0]["a"]) assert row["b"] == str(data[0]["b"])
def test_export_json_creates_file(tmp_path): data = [{"x": 10}] file_path = tmp_path / "data.json" export_to_json(data, str(file_path)) assert file_path.exists()
mariobehling
commented
Feb 8, 2026
Small process note.
We have automatic Copilot PR reviews enabled on this repository. These reviews are only triggered if the contributor has GitHub Copilot enabled and an active license on their own account.
Please enable Copilot in your GitHub settings if you have access. In many regions, free licenses are available through educational institutions or developer programs. Enabling Copilot helps us speed up the auto review process and reduces manual review overhead for the core team.
The team will review your PR. Thank you for your contribution and cooperation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot flagged the following:
- Tests-only companion to #260 — CI fails if merged alone.
Uh oh!
There was an error while loading. Please reload this page.
This PR adds utility functions to export PSLab sensor data
to CSV and JSON formats.
This improves data usability for PSLab users.
Summary by Sourcery
Add utilities for exporting PSLab sensor data to common file formats and ensure they are covered by basic tests.
New Features:
Tests: