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

Document CellKey being a RowKey, ColumnKey tuple #3624

Merged
merged 2 commits into from
Nov 2, 2023

Conversation

pawamoy
Copy link
Contributor

@pawamoy pawamoy commented Nov 1, 2023

Please review the following checklist.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md (where appropriate)

Following #3623.

An alternative is to add docstrings to the row_key and column_key attributes of the CellKey class. (UPDATE: also added)

@@ -131,6 +131,8 @@ class ColumnKey(StringKey):
class CellKey(NamedTuple):
"""A unique identifier for a cell in the DataTable.

A cell key is a `(row_key, column_key)` tuple.
Copy link
Contributor

Choose a reason for hiding this comment

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

As per your main comment, I think if it were me I'd be tempted to do both: make a note to this effect here, but also add docstrings to the two properties as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's perfect. :-)

@davep davep merged commit 3bb8c46 into Textualize:main Nov 2, 2023
22 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