Commit d028005
Add comprehensive unit tests for DataFrame operations and
Adds test coverage gaps identified in the PR #98 review: direct tests
for `_normalize_scalar()` and an end-to-end mocked CRUD flow for
`DataFrameOperations`.
## `tests/unit/test_pandas_helpers.py`
- New `TestNormalizeScalar` class (9 tests) directly exercising
`_normalize_scalar()`:
- NumPy types (`np.integer`, `np.floating`, `np.bool_`) → Python natives
- `pd.Timestamp` → ISO 8601 string
- Native Python types and `None` pass through unchanged
## `tests/unit/test_dataframe_operations.py`
- New `TestDataFrameEndToEnd` class (2 tests):
- Full mocked CRUD cycle: `create → get → update → delete`
- Verifies NumPy types are normalized to Python-native values before
reaching the API layer
## Notes
- `filter` parameter kept as-is (consistent with `records.get()` API;
repo convention prohibits `# noqa` suppression)
- `DataFrameOperations` not re-exported from top-level `__init__.py`
(repo convention: package `__init__.py` files use `__all__ = []`)
<!-- START COPILOT ORIGINAL PROMPT -->
<details>
<summary>Original prompt</summary>
## Context
This PR addresses the remaining unresolved review comments from PR #98
(#98)
and adds comprehensive unit tests for the DataFrame operations.
The PR #98 adds DataFrame CRUD wrappers (`client.dataframe.get()`,
`client.dataframe.create()`, `client.dataframe.update()`,
`client.dataframe.delete()`) to the Dataverse Python SDK. The author has
addressed many review comments but several remain unresolved.
## Current State of the Code
The branch `users/zhaodongwang/dataFrameExtensionClaude` has the latest
code. Key files:
### `src/PowerPlatform/Dataverse/utils/_pandas.py` (current)
```python
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT license.
"""Internal pandas helpers"""
from __future__ import annotations
from typing import Any, Dict, List
import numpy as np
import pandas as pd
def _normalize_scalar(v: Any) -> Any:
"""Convert numpy scalar types to their Python native equivalents."""
if isinstance(v, pd.Timestamp):
return v.isoformat()
if isinstance(v, np.integer):
return int(v)
if isinstance(v, np.floating):
return float(v)
if isinstance(v, np.bool_):
return bool(v)
return v
def dataframe_to_records(df: pd.DataFrame, na_as_null: bool = False) -> List[Dict[str, Any]]:
"""Convert a DataFrame to a list of dicts, normalizing values for JSON serialization."""
records = []
for row in df.to_dict(orient="records"):
clean = {}
for k, v in row.items():
if pd.api.types.is_scalar(v):
if pd.notna(v):
clean[k] = _normalize_scalar(v)
elif na_as_null:
clean[k] = None
else:
clean[k] = v
records.append(clean)
return records
```
### `src/PowerPlatform/Dataverse/operations/dataframe.py` (current - 305
lines)
The `DataFrameOperations` class provides get/create/update/delete
methods. Key points:
- `get()` returns a single consolidated DataFrame (iterates all pages
internally)
- `create()` validates non-empty, validates ID count matches
- `update()` validates id_column exists, validates IDs are non-empty
strings, validates at least one change column exists; has `clear_nulls`
parameter
- `delete()` validates ids is Series, validates IDs are non-empty
strings, special-cases single ID
### `src/PowerPlatform/Dataverse/operations/__init__.py` (current)
```python
from .dataframe import DataFrameOperations
__all__ = ["DataFrameOperations"]
```
### `src/PowerPlatform/Dataverse/__init__.py` (current)
```python
from importlib.metadata import version
__version__ = version("PowerPlatform-Dataverse-Client")
__all__ = ["__version__"]
```
### `src/PowerPlatform/Dataverse/client.py` (current)
Already imports and exposes `DataFrameOperations` as `self.dataframe`.
## Issues to Fix
### 1. `filter` parameter shadows Python built-in (item #8)
In `dataframe.py` `get()` method, the parameter `filter` shadows the
Python built-in `filter()`. Since this mirrors the existing
`records.get()` API which also uses `filter`, renaming is risky for API
consistency. The safe fix is to add a `# noqa: A002` comment on the
parameter and leave it as-is for API consistency (the base
`records.get()` already uses `filter`). Alternatively, rename to
`filter_expr` with an alias for backward compatibility. **Decision: keep
`filter` for API consistency with existing `records.get()`, but suppress
the lint warning.**
### 2. Missing `__init__.py` export for `DataFrameOperations` (item #9)
The `operations/__init__.py` already exports `DataFrameOperations`.
However, the top-level `src/PowerPlatform/Dataverse/__init__.py` does
NOT export it. Add the export there so users can do `from
PowerPlatform.Dataverse import DataFrameOperations` if needed.
### 3. Comprehensive unit tests (item #10)
The existing `tests/unit/test_client_dataframe.py` has 365 lines of
tests. We need to add MORE tests to ensure full coverage. Specifically
add tests for:
**Unit tests for `_pandas.py` helpers:**
- `_normalize_scalar` with np.int64, np.float64, np.bool_, pd.Timestamp,
regular Python types
- `dataframe_to_records` with NaN handling (na_as_null=True vs False)
- `dataframe_to_records` with Timestamp conversion
- `dataframe_to_records` with non-scalar values (lists, dicts in cells)
- `dataframe_to_records` with numpy scalar types in DataFrame
- `dataframe_to_records` with empty DataFrame
- `dataframe_to_records` with mixed types
**Unit tests for `DataFrameOperations`:**
- `get()` single record
- `get()` multi-page results concatenated
- `get()` empty results
- `get()` with all parameters passed through
- `create()` with valid DataFrame
- `create()` with empty DataFrame (should raise ValueError)
- `create()` with non-DataFrame input (should raise TypeError)
- `create()` ID count mismatch (should raise ValueError)
- `update()` with valid DataFrame
- `update()` single record path
- `...
</details>
<!-- START COPILOT CODING AGENT SUFFIX -->
*This pull request was created from Copilot chat.*
>
<!-- START COPILOT CODING AGENT TIPS -->
---
🔒 GitHub Advanced Security automatically protects Copilot coding agent
pull requests. You can protect all pull requests by enabling Advanced
Security for your repositories. [Learn more about Advanced
Security.](https://gh.io/cca-advanced-security)
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: saurabhrb <32964911+saurabhrb@users.noreply.github.com>_normalize_scalar (#146)1 parent 4809dca commit d028005
2 files changed
Lines changed: 130 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
361 | 361 | | |
362 | 362 | | |
363 | 363 | | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
364 | 437 | | |
365 | 438 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
12 | 68 | | |
13 | 69 | | |
14 | 70 | | |
| |||
0 commit comments