close
Skip to content

Fix TypeError when stacking two headerless Datasets with stack_cols - #651

Merged
claudep merged 1 commit into
jazzband:masterfrom
vineethsaivs:fix/stack-cols-headerless
Jul 16, 2026
Merged

Fix TypeError when stacking two headerless Datasets with stack_cols#651
claudep merged 1 commit into
jazzband:masterfrom
vineethsaivs:fix/stack-cols-headerless

Conversation

@vineethsaivs

Copy link
Copy Markdown
Contributor

Problem

Dataset.stack_cols() crashes with an opaque TypeError when both datasets have no headers, even though the method is written to allow that case:

>>> import tablib
>>> a = tablib.Dataset(); a.append(("x", "y")); a.append(("p", "q"))
>>> b = tablib.Dataset(); b.append(("1", "2", "3")); b.append(("4", "5", "6"))
>>> a.stack_cols(b)
TypeError: 'NoneType' object is not iterable

The method already anticipates two headerless datasets:

  • the header guard raises HeadersNeeded only when exactly one side has headers, and
  • the header concatenation catches None + None and sets new_headers = None.

But it then does for column in self.headers: to copy the columns, which iterates None and raises. Row-level stack() handles headerless datasets fine, so the column-level crash is an internal inconsistency rather than intended behavior.

Fix

When neither dataset has headers, copy the columns by index with get_col() instead of by header name. The existing headered path is unchanged (it is just moved under if self.headers:), so headered stacking behaves exactly as before.

Test

Added test_column_stacking_headerless, which stacks two headerless datasets and asserts the combined rows and headers is None. It fails on the current code with the TypeError above and passes with this change; the existing test_column_stacking (headered) still passes.

Note

This touches the same method as the open PR #649 ("Raise TypeError for invalid Dataset instance"), but a different line: #649 changes the isinstance(other, Dataset) guard, while this fixes the headerless column-iteration path. The two do not overlap and either order rebases trivially.

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.42%. Comparing base (163f5e1) to head (116ba2f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #651      +/-   ##
==========================================
+ Coverage   93.32%   93.42%   +0.09%     
==========================================
  Files          29       29              
  Lines        3282     3298      +16     
==========================================
+ Hits         3063     3081      +18     
+ Misses        219      217       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claudep

claudep commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Could you please rebase your PR on latest master?

Dataset.stack_cols() explicitly permits two headerless datasets: the
header guard only raises HeadersNeeded when exactly one side has headers,
and the header concatenation catches None + None to set new_headers=None.
But it then did 'for column in self.headers', which iterates None and
raised 'TypeError: NoneType object is not iterable', so column-level
stacking crashed on inputs that row-level stack() handles fine.

When neither dataset has headers, iterate columns by index with get_col()
instead of by header name. The headered path is unchanged.
@vineethsaivs
vineethsaivs force-pushed the fix/stack-cols-headerless branch from 862a731 to 116ba2f Compare July 15, 2026 22:03
@vineethsaivs

Copy link
Copy Markdown
Contributor Author

Thanks! Rebased onto the latest master (it now cleanly picks up the new test_stack_rejects_non_dataset test alongside this fix's test_column_stacking_headerless). Should be mergeable now.

@claudep claudep added the changelog: Fixed For any bug fixes label Jul 16, 2026

@claudep claudep left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@claudep
claudep merged commit 234f434 into jazzband:master Jul 16, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: Fixed For any bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants