Skip to content

Raise TypeError when scalar column added to empty table - #10476

Merged
bsipocz merged 6 commits into
astropy:masterfrom
gregosimo:columnscalar
Oct 23, 2020
Merged

bsipocz merged 6 commits into
astropy:masterfrom
gregosimo:columnscalar

Conversation

@gregosimo

Copy link
Copy Markdown

Description

This pull request is to address the uninformative error message when a column in an empty table is assigned to a scalar. Now assigning a column in an empty table to a scalar will result in an TypeError at assignment with a relevant error message, rather than a TypeError on access with a more opaque error message.

Fixes #3811

@astropy-bot astropy-bot Bot added the table label Jun 9, 2020
Comment thread CHANGES.rst Resolved
@pllim pllim added the API change PRs and issues that change an existing API, possibly requiring a deprecation period label Jun 9, 2020
@pllim pllim added this to the v4.2 milestone Jun 9, 2020
@pllim
pllim requested review from mhvk and taldcroft June 9, 2020 16:20
Comment thread astropy/table/table.py Resolved
Comment thread astropy/table/tests/test_table.py Resolved
Comment thread CHANGES.rst Resolved
@taldcroft taldcroft changed the title Raise TypeError when scalar column added to empty table [fix for #3811] Raise TypeError when scalar column added to empty table Jun 11, 2020
@taldcroft

taldcroft commented Jun 11, 2020 •

Copy link
Copy Markdown
Member

@gregosimo - looks generally good, thanks! I made a few tiny tweaks.

@taldcroft

Copy link
Copy Markdown
Member

I see now that the changelog entry is in the wrong section. It is currently in 4.0.1 but should be in 4.2.

@bsipocz bsipocz added the zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now. label Oct 23, 2020

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I take the discussion as an approval from the package maintainer, so let's just get this merged now.

@bsipocz
bsipocz merged commit 83708c5 into astropy:master Oct 23, 2020
@bsipocz

bsipocz commented Oct 23, 2020

Copy link
Copy Markdown
Member

Thank you @gregosimo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API change PRs and issues that change an existing API, possibly requiring a deprecation period table zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better error if adding a non-iterable object to a table as a column

5 participants