Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add method for setting a column's value #42

Merged
merged 6 commits into from
Aug 18, 2023
Merged

Add method for setting a column's value #42

merged 6 commits into from
Aug 18, 2023

Conversation

spenczar
Copy link
Collaborator

@spenczar spenczar commented Aug 17, 2023

This was previously possible through direct access, but this is a lot clearer and cleaner. It makes it much more obvious that this happens as a copy, not in-place.

TODO:

  • Write docs
  • Update changelog
  • Revisit how Column.__set__ is implemented, possibly discarding it entirely

@spenczar spenczar requested a review from moeyensj August 17, 2023 23:07
@spenczar
Copy link
Collaborator Author

The doc build failure is due to pradyunsg/furo#693, and is spurious; it should be fixed once readthedocs is using Sphinx 7.2.2.

Copy link
Member

@moeyensj moeyensj left a comment

Choose a reason for hiding this comment

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

I've tested this with some adam_core examples and it works! This really makes it a lot clearer that we get a modified copy back.

What do you think about setting nullable columns to None? Table.set_column("name", None)? If I try an operation like that I get:

TypeError: 'NoneType' object is not iterable

This was previously possible through direct access, but this is a lot
clearer and cleaner. It makes it much more obvious that this happens
as a copy, not in-place.
@spenczar spenczar merged commit a914b6f into main Aug 18, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants