Skip to content

Added AttributeError when trying to set mask on non-masked table - #4637

Merged
taldcroft merged 1 commit into
astropy:masterfrom
mvnnn:bugfix-for-issue-3505
Mar 1, 2016
Merged

taldcroft merged 1 commit into
astropy:masterfrom
mvnnn:bugfix-for-issue-3505

Conversation

@mvnnn

@mvnnn mvnnn commented Feb 26, 2016

Copy link
Copy Markdown
Contributor

fixes #3505

@mwcraig mwcraig added the table label Feb 26, 2016
@mvnnn

mvnnn commented Feb 29, 2016

Copy link
Copy Markdown
Contributor Author

ping @taldcroft

Comment thread astropy/table/column.py Outdated

def __setattr__(self, item, value):
if not isinstance(self, MaskedColumn) and item == "mask":
raise AttributeError("Cannot set mask value to a Column in non-masked Table")

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.

Use lower case to start the error message, i.e. cannot set ... We are not consistent about this but new code should follow that convention.

@taldcroft taldcroft self-assigned this Feb 29, 2016
@taldcroft taldcroft added this to the v1.2.0 milestone Feb 29, 2016
@taldcroft

Copy link
Copy Markdown
Member

@mvnnn - please add an entry to the new features for 1.2 and address my minor comments, then this will be good. Thanks!

@taldcroft

Copy link
Copy Markdown
Member

(I meant new features in CHANGES.rst).

@mvnnn
mvnnn force-pushed the bugfix-for-issue-3505 branch from 1c690db to ca81b1b Compare February 29, 2016 16:11
@mvnnn
mvnnn force-pushed the bugfix-for-issue-3505 branch from ca81b1b to 7c31248 Compare February 29, 2016 16:15
@taldcroft

Copy link
Copy Markdown
Member

@mvnnn - it is not necessary or desirable to always squash commits into one. By doing so that makes it more difficult to see that you made the requested changes. I just restarted a failed build test and hopefully that will give you a CI pass.

taldcroft added a commit that referenced this pull request Mar 1, 2016
Added AttributeError when trying to set mask on non-masked table
@taldcroft
taldcroft merged commit 2c7db48 into astropy:master Mar 1, 2016
@taldcroft

Copy link
Copy Markdown
Member

Thanks @mvnnn !

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Setting mask on non-masked column should raise error

3 participants