[dsfulf]

Summary

• Cross join pair generation: fix np.tile/np.repeat to produce correct Cartesian product pairs
• Dtype promotion: use np.result_type to promote to highest precision (e.g. int32 + int64 → int64) instead of rejecting mismatched precisions
• Null key handling: filter None/NaN/NaT from join keys before matching (SQL semantics: NULL != NULL), re-insert null-key left rows in left joins
• Empty table joins: short-circuit with correct empty result + warning instead of ValueError
• Cross join column precedence: left table wins on name conflicts (matches inner/left join behavior)
• Left join null-fill: StringDType columns use StringDType(na_object=None) with None, float uses NaN, datetime/timedelta uses NaT, int/bool/bytes falls back to object with warning
• Method placement: moved _left_join_indices from base Join to LeftJoin
• Test reorganization: grouped all tests into classes by category (TestConstruction, TestJoins, TestGroupBy, etc.)
• Comprehensive join tests: 50+ new tests covering all join types × operators × dtype combinations × null keys × empty tables
• Docs: new "Null Handling & Dtype Behavior" section in joins.md documenting type promotion via np.result_type, null-fill semantics, and null key behavior

Test plan

• pytest — all tests pass
• ruff format + ruff check — clean
• mypy --strict tafra — no new errors
• Cross join pair correctness verified against manual Cartesian product
• Null key filtering verified: inner join excludes, left join preserves left-side nulls
• Empty table joins return correct schema with zero rows
• Dtype promotion: int32×int64→int64, float32×float64→float64, StringDType×<U→StringDType

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR focuses on improving join correctness and documented semantics in Tafra, especially for cross joins, null join keys, empty-table joins, and dtype compatibility/promotion, alongside a major expansion/reorganization of the test suite.

Changes:

• Fix cross-join Cartesian product pairing logic and define left-precedence behavior on name conflicts.
• Add join behavior for null keys and empty-table joins, plus expanded documentation of null/dtype semantics.
• Reorganize and significantly expand join-related tests (and broader suite) for coverage across operators, dtypes, nulls, and empty inputs.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File	Description
test/test_tafra.py	Reorganized tests into classes and added extensive new join/dtype/null/empty-table test coverage.
tafra/group.py	Updated join implementations (null-key filtering, empty shortcuts, cross join pairing) and adjusted dtype validation logic.
docs/guide/joins.md	Added a detailed “Null Handling & Dtype Behavior” section and updated left-join docs to reference it.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

_validate_dtypes uses Tafra._reduce_dtype() to allow “same base type” before the numeric-promotion branch. Since _reduce_dtype('int32') == _reduce_dtype('int64') == 'int' (and similarly for float widths), this makes the np.result_type promotion block effectively unreachable for width mismatches. If the intent is to promote int/float precisions, the base-type shortcut should be limited to string-like dtypes (or use a reducer that only collapses string variants).

Suggested change

	# Check base type via _reduce_dtype (collapses str variants)
	l_base = Tafra._reduce_dtype(l_dtype)
	r_base = Tafra._reduce_dtype(r_dtype)
	if l_base == r_base:
	# Same base type (e.g. both 'str') — allow
	# Check base type via _reduce_dtype (intended to collapse str variants)
	l_base = Tafra._reduce_dtype(l_dtype)
	r_base = Tafra._reduce_dtype(r_dtype)
	# Only treat string-like columns with the same reduced dtype as compatible
	# without further checks; allow numeric types to fall through so they can
	# be promoted via np.result_type when widths differ.
	if l_base == r_base == 'str':
	# Same base string type (e.g. 'str32' vs 'str64') — allow

[Copilot]

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (1)

tafra/group.py:505

• Docstring for GroupSet._build_composite_key says it “Falls back to structured array if values would overflow int64”, but the implementation now raises ValueError instead. Update the docstring to reflect the new behavior (or reintroduce the structured-array fallback if that was still intended).

    def _build_composite_key(encoded: list[np.ndarray[Any, Any]]) -> np.ndarray[Any, Any]:
        """
        Combine multiple integer-coded columns into a single flat key.
        Uses positional encoding: key = c0 * N1*N2*... + c1 * N2*... + c2 * ...
        Falls back to structured array if values would overflow int64.
        """

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

_validate_dtypes promotes numeric join-key dtypes by casting and updating _data/_dtypes on the input Tafra objects. This introduces a surprising side effect (joins mutating their inputs) and can affect subsequent operations on the original tables. Consider promoting/casting into temporary key arrays (or shallow copies of just the key columns) used for matching, and keep the original left_t/right_t unchanged.

Suggested change

	continue
	# Collapse string variants: StringDType, <U8, <U12 all reduce to 'str'
	l_base = Tafra._reduce_dtype(l_dtype)
	r_base = Tafra._reduce_dtype(r_dtype)
	if l_base == r_base == "str":
	# Both are string types — allow without promotion
	continue
	# Check numpy kind compatibility for numeric promotion
	l_np = np.dtype(l_dtype)
	r_np = np.dtype(r_dtype)
	if l_np.kind == r_np.kind:
	# Same family (e.g. int32 vs int64) — promote via result_type
	promoted = np.result_type(l_np, r_np)
	promoted_label = Tafra._format_dtype(promoted)
	l_table._data[l_column] = l_table._data[l_column].astype(promoted)
	l_table._dtypes[l_column] = promoted_label
	r_table._data[r_column] = r_table._data[r_column].astype(promoted)
	r_table._dtypes[r_column] = promoted_label
	# Exact dtype match — nothing to validate further.
	continue
	# Collapse string variants: StringDType, <U8, <U12 all reduce to 'str'
	l_base = Tafra._reduce_dtype(l_dtype)
	r_base = Tafra._reduce_dtype(r_dtype)
	if l_base == r_base == "str":
	# Both are string types — allow without promotion or mutation.
	continue
	# Check numpy kind compatibility for non-string types
	l_np = np.dtype(l_dtype)
	r_np = np.dtype(r_dtype)
	if l_np.kind == r_np.kind:
	# Same family (e.g. int32 vs int64, float32 vs float64, etc.).
	# Treat as compatible as long as there is a safe cast in at least
	# one direction. Do not mutate the original Tafra tables here.
	if not (
	np.can_cast(l_np, r_np, casting="safe")
	or np.can_cast(r_np, l_np, casting="safe")
	):
	raise TypeError(
	f"This `Tafra` column `{l_column}` dtype `{l_dtype}` "
	f"cannot be safely cast to or from other `Tafra` dtype "
	f"`{r_dtype}`."
	)

[Copilot]

_non_null_mask only treats None as null for object-dtype key columns, but the PR description/docs also claim NaN/NaT are filtered. For object arrays containing np.nan or np.datetime64('NaT'), those rows will currently be considered valid and may incorrectly match (especially with !=). Update the object-path to also exclude NaN and NaT values.

Suggested change

	elif kind in ("M", "m"):
	# Object-dtype: treat None, NaN, and NaT as nulls
	def _is_valid_obj(x: Any) -> bool:
	if x is None:
	return False
	# Treat NaN as null for numeric-like objects
	if isinstance(x, (float, np.floating)):
	try:
	if np.isnan(x):
	return False
	except TypeError:
	# Non-numeric objects may raise here; consider them non-null
	pass
	# Treat NaT as null for datetime/timedelta-like objects
	if isinstance(x, (np.datetime64, np.timedelta64)):
	if np.isnat(x):
	return False
	return True
	valid &= np.fromiter((_is_valid_obj(x) for x in c), dtype=bool, count=n)

[Copilot]

The left-join empty-table shortcuts return before validating on columns/operators. That means invalid join specifications may not raise when left_t or right_t has zero rows. Consider performing the usual _validate(...)/_validate_dtypes(...)/_validate_ops(...) checks (at least for schema) before returning the shortcut result.

[Copilot]

Same issue in the left-join row-by-row fallback: without explicit null filtering, null keys (e.g. NaN) can match many right rows for != and produce non-SQL semantics. Apply the same null-key exclusion logic used in the equi-join path (or otherwise treat any null comparison as non-matching) in this fallback loop.

Suggested change

	left_val = left_t._data[left_col][i]
	# Treat any null/NaN left key as non-matching for this row
	is_left_null = left_val is None
	if not is_left_null:
	try:
	# np.isnan handles float/np.nan; will raise for non-numerics
	is_left_null = bool(np.isnan(left_val)) # type: ignore[arg-type]
	except (TypeError, ValueError):
	is_left_null = False
	if is_left_null:
	right_rows[:] = False
	break
	right_col_data = right_t._data[right_col]
	# Compute null mask for right-hand join column and exclude nulls
	try:
	right_null_mask = np.isnan(right_col_data)
	except (TypeError, ValueError):
	right_null_mask = np.zeros(right_col_data.shape, dtype=bool)
	cmp_result = op(left_val, right_col_data)
	right_rows &= cmp_result & ~right_null_mask

[Copilot]

CrossJoin.apply builds the result via Tafra(result) without passing the merged _dtypes metadata, so user-declared dtypes (and string handling conventions) may be lost compared to Inner/Left joins. Also, the current dict construction inserts right-only columns before left columns, which differs from Inner/Left join column ordering. Consider using _resolve_join_cols(...) to supply consistent dtypes, and build the result dict in left-then-right order while still letting left win on name conflicts.

[Copilot]

Docs claim “Column order follows the select list”, but Inner/Left joins build join_cols by iterating table column order and filtering by membership (_resolve_join_cols), so the output column order is table-defined, not select-defined. Either update the implementation to honor select ordering or adjust this documentation and example to reflect current behavior.

[Copilot]

ri_mapped = np.where(ri >= 0, right_orig_idx[ri], -1) can raise when right_orig_idx is empty (e.g. all right-side join keys are null so right_valid filters everything out) because right_orig_idx[ri] is evaluated even when all ri are -1. Build ri_mapped via masked assignment (initialize to -1, then fill only ri >= 0) to avoid indexing with -1 into an empty array.

Suggested change

	else:
	# Initialize all to -1, then fill only positions where ri >= 0
	ri_mapped = np.full_like(ri, -1, dtype=np.intp)
	nonneg_mask = ri >= 0
	if np.any(nonneg_mask):
	ri_mapped[nonneg_mask] = right_orig_idx[ri[nonneg_mask]]

[Copilot]

The empty-table shortcut returns before validating on columns and operators. This can silently hide user errors (e.g., joining on a missing column) whenever either side has zero rows. Consider still running _validate(...), _validate_dtypes(...), and _validate_ops(...) before returning the shortcut result so schema mistakes are caught consistently.

[Copilot]

Null-key filtering is only applied in the all_equi fast path. In the row-by-row fallback, rows with NaN/NaT/None keys can still match (notably NaN != NaN evaluates True), which violates the stated SQL-like NULL semantics. The fallback path should explicitly exclude null-key rows from matching for all operators.

[Copilot]

The table implies <U8 + <U12 join keys yield a “Result dtype” of <U12, but the join implementation does not promote/cast string key columns (it only allows them to join). Depending on column naming/precedence, the output dtype may remain the left dtype. Consider clarifying what “Result dtype” refers to here (matching key encoding vs output column dtype), or adjust the statement to match actual behavior.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Join._non_null_mask doesn’t correctly detect nulls in object key columns: it only treats isinstance(x, float) and np.isnan(x) as null, which misses common cases like np.float64(np.nan) (np.floating) and np.datetime64('NaT')/np.timedelta64('NaT') stored in object arrays. This can allow NULL keys to participate in matches (e.g., NaT == NaT), violating the documented SQL-like semantics. Consider expanding the object-path predicate to handle np.floating NaNs and np.isnat(...) for datetime/timedelta scalars (and/or vectorize with np.asarray(c, dtype=...) where possible).

Suggested change

	# Object arrays can hold None, NaN, and NaT simultaneously
	valid &= np.array(
	[x is not None and not (isinstance(x, float) and np.isnan(x)) for x in c],
	dtype=bool,
	)
	# Object arrays can hold None, NaN (including np.floating),
	# and NaT (np.datetime64/np.timedelta64) simultaneously.
	def _is_object_null(x: Any) -> bool:
	if x is None:
	return True
	# NaN in Python float or numpy.floating
	if isinstance(x, (float, np.floating)) and np.isnan(x):
	return True
	# NaT for datetime64/timedelta64 scalars stored in object arrays
	if isinstance(x, (np.datetime64, np.timedelta64)):
	try:
	if np.isnat(x):
	return True
	except TypeError:
	# np.isnat may not accept some scalar-like objects; treat as non-null
	pass
	return False
	valid &= np.array([not _is_object_null(x) for x in c], dtype=bool)

[Copilot]

CrossJoin select handling can produce an empty left_cols or right_cols (e.g., select=['only_right_col']). The current code then calls left_t[left_cols] / right_t[right_cols], which constructs a Tafra with no columns and fails in Tafra.__post_init__ (“No data provided…”). Either explicitly error with a clear message when one side selects zero columns, or support this case by skipping the empty-side selection and only repeating/tiling the non-empty side.

Suggested change

	left_data = dict(left_t[left_cols].key_map(np.repeat, repeats=right_rows))
	right_data = dict(right_t[right_cols].key_map(np.tile, reps=left_rows))
	# Handle cases where select leaves one side with no columns:
	# avoid constructing an empty-column Tafra, which would fail __post_init__.
	if left_cols:
	left_data = dict(
	left_t[left_cols].key_map(np.repeat, repeats=right_rows)
	)
	else:
	left_data = {}
	if right_cols:
	right_data = dict(
	right_t[right_cols].key_map(np.tile, reps=left_rows)
	)
	else:
	right_data = {}

[Copilot]

CrossJoin output column order is reversed: result is populated with (non-conflicting) right columns first, then result.update(left_data) appends left columns last. This contradicts the stated behavior elsewhere (left-table columns first, then right-table columns) and differs from update_inplace behavior used previously. Build the result dict starting from left_data and then append non-conflicting right_data to preserve left→right ordering.

Suggested change

	result: dict[str, np.ndarray[Any, Any]] = {}
	for c, v in right_data.items():
	if c not in left_data:
	result[c] = v
	result.update(left_data)
	# Start with left_data to preserve left→right column ordering, then append
	# only non-conflicting right columns.
	result: dict[str, np.ndarray[Any, Any]] = dict(left_data)
	for c, v in right_data.items():
	if c not in result:
	result[c] = v

[Copilot]

LeftJoin empty-right shortcut fills non-nullable right columns by casting to object and setting values to None, but it doesn’t emit the warning that the main left-join path emits when it has to cast to object due to missing native nulls. This makes the empty-table behavior inconsistent with non-empty joins and with the docs’ stated warning behavior. Consider warning here as well when falling back to object.

[Copilot]

The “Dtype validation on join keys” section says mixed-precision numeric keys are “promoted” and the “Result dtype” becomes the promoted dtype (e.g., int32 + int64 → int64). In the current implementation, the promotion is only used to compute temporary join keys; the output columns keep their original dtypes (and if the join key name is shared, the left column wins). Please clarify in the docs whether promotion affects only matching or also the resulting column dtypes, and adjust the table/examples accordingly.

Suggested change

	- **Same-family numeric types are promoted**: `int32` vs `int64`, or
	`float32` vs `float64`, are promoted to the higher precision type using
	`np.result_type`. The narrower column is cast automatically.
	- **Different families are rejected**: `int64` vs `float64`, or `int64` vs
	`str`, raise `TypeError`.
	| Left key dtype | Right key dtype | Allowed? | Result dtype |
	|---|---|---|---|
	| `StringDType` | `<U10` | Yes | both keep original dtypes |
	| `<U8` | `<U12` | Yes | both keep original dtypes |
	| `int32` | `int64` | Yes (promoted) | `int64` |
	| `float32` | `float64` | Yes (promoted) | `float64` |
	| `int32` | `int32` | Yes | `int32` |
	| `int64` | `float64` | No — `TypeError` | — |
	| `int64` | `StringDType` | No — `TypeError` | — |
	| `datetime64[ns]` | `datetime64[us]` | Yes | both keep original dtypes |
	Type promotion uses `np.result_type` under the hood — the same function numpy
	uses to determine output dtypes for binary operations. Users familiar with
	numpy's promotion rules will find no surprises here.
	```python
	# int32 key joins with int64 key — promoted automatically
	left = Tafra({'k': np.array([1, 2], dtype=np.int32), 'v': np.array([10, 20])})
	right = Tafra({'k': np.array([1, 2], dtype=np.int64), 'info': np.array(['a', 'b'])})
	result = left.inner_join(right, on=[('k', 'k', '==')])
	# Works — k promoted to int64
	- **Same-family numeric types are promoted for matching**: `int32` vs
	`int64`, or `float32` vs `float64`, are promoted to the higher precision
	type using `np.result_type` to compute temporary join keys. The underlying
	columns in the result keep their original dtypes; if the join key name is
	shared, the left column (and its dtype) wins.
	- **Different families are rejected**: `int64` vs `float64`, or `int64` vs
	`str`, raise `TypeError`.
	| Left key dtype | Right key dtype | Allowed? | Result dtype (output columns) |
	|---|---|---|---|
	| `StringDType` | `<U10` | Yes | both keep original dtypes |
	| `<U8` | `<U12` | Yes | both keep original dtypes |
	| `int32` | `int64` | Yes (promoted for matching) | both keep original dtypes; left wins on name clash |
	| `float32` | `float64` | Yes (promoted for matching) | both keep original dtypes; left wins on name clash |
	| `int32` | `int32` | Yes | `int32` (left wins on name clash) |
	| `int64` | `float64` | No — `TypeError` | — |
	| `int64` | `StringDType` | No — `TypeError` | — |
	| `datetime64[ns]` | `datetime64[us]` | Yes | both keep original dtypes |
	Type promotion uses `np.result_type` under the hood — the same function numpy
	uses to determine output dtypes for binary operations. Promotion only affects
	the temporary keys used for matching; the dtypes of the columns that appear
	in the joined result are not changed.
	```python
	# int32 key joins with int64 key — promoted automatically for matching
	left = Tafra({'k': np.array([1, 2], dtype=np.int32), 'v': np.array([10, 20])})
	right = Tafra({'k': np.array([1, 2], dtype=np.int64), 'info': np.array(['a', 'b'])})
	result = left.inner_join(right, on=[('k', 'k', '==')])
	# Works — join keys are temporarily promoted, but result['k'] keeps the left dtype (int32)

[Copilot]

_validate_dtypes returns a single promotions dict keyed only by column name, but join conditions can reference the same column name on different sides across multiple predicates (e.g. on=[('a','b','=='), ('b','c','==')] where 'b' is right-side in one predicate and left-side in another). In that case promotions['b'] gets overwritten and the wrong dtype can be applied when building key arrays, producing incorrect matches. Consider returning separate promotion maps for left vs right (or keying promotions by (side, column)), and use those when casting left_cols_data / right_cols_data.

Suggested change

	promotions[l_column] = promoted
	promotions[r_column] = promoted
	# Record promotions for each side, detecting conflicts when the
	# same column name participates in multiple join predicates.
	for col_name in (l_column, r_column):
	existing = promotions.get(col_name)
	if existing is None:
	promotions[col_name] = promoted
	elif existing != promoted:
	raise TypeError(
	"Inconsistent dtype promotion for join key "
	f"column `{col_name}` across multiple join "
	f"predicates: previously promoted to `{existing}` "
	f"but now would promote to `{promoted}`."
	)

[Copilot]

GroupSet._build_composite_key docstring says it “Falls back to structured array if values would overflow int64”, but the implementation now raises ValueError on overflow. This is both a behavioral regression (previously large multi-key groupby/join could still work via slower fallback) and a docs mismatch. Consider restoring the structured-array fallback (and bypassing the C accel path when the key isn’t int64) or updating the docstring and documenting the new hard limit.