Documentation added for Rows class. #552
Conversation
datascience/tables.py
Outdated
| args: | ||
| table- accepts a table object of class Table() | ||
|
|
||
| >>> t = td.Table().with_columns({ |
There was a problem hiding this comment.
I'm unsure why we're not instantiating the Table class directly here. What's the td object here? I don't see it anywhere in the docs?
There was a problem hiding this comment.
That was a mistake on my side. I didn't add the proper initialization for Table() class. I have updated it now.
datascience/tables.py
Outdated
| 'points': [ 1, 2, 2, 10], | ||
| }) | ||
|
|
||
| >>> td.Table().Rows(t).__getitem__(0) |
There was a problem hiding this comment.
You can reuse objects from previous docs. Let's save one Rows object in the class initializer and reuse that for the remaining methods in this class.
datascience/tables.py
Outdated
| @@ -5735,9 +5771,35 @@ def __getitem__(self, i): | |||
| return self._row(c[i] for c in self._table._columns.values()) | |||
|
|
|||
| def __len__(self): | |||
| """ | |||
| Returns the no.of rows in the table. | |||
There was a problem hiding this comment.
please no short-forms in the docs, as much as possible
datascience/tables.py
Outdated
| 'count': [ 9, 3, 3, 1], | ||
| 'points': [ 1, 2, 2, 10], | ||
| }) | ||
| >>> td.Table().Rows(t).__len__() |
There was a problem hiding this comment.
We can jus use the Python len function to demonstrate this function's logic
datascience/tables.py
Outdated
| @@ -5719,12 +5719,48 @@ def asdict(self): | |||
| return collections.OrderedDict(zip(self._table.labels, self)) | |||
|
|
|||
| class Rows(collections.abc.Sequence): | |||
| """An iterable view over the rows in a table.""" | |||
| """ | |||
| An iterable row-styled view of the table object that is passed as an argument. | |||
There was a problem hiding this comment.
Let's keep the original statement here - the intended/standard usage of this class doesn't have users manually creating this class.
|
@adnanhemani I have updated the docs according to the changes suggested. |
datascience/tables.py
Outdated
| args: | ||
| table- accepts a table object of class Table() | ||
|
|
||
| >>> from datascience import Table |
There was a problem hiding this comment.
Table should already be imported from other doctests above, no need to re-import it.
datascience/tables.py
Outdated
| }) | ||
|
|
||
| >>> r = Table.Rows(t) | ||
| print(r) |
There was a problem hiding this comment.
In doctests, we don't need to print any values. You can simply call >>> r and it will output the right representation of the object.
datascience/tables.py
Outdated
| An iterable view over the rows in a table. | ||
|
|
||
| args: | ||
| table- accepts a table object of class Table() |
There was a problem hiding this comment.
Can we please follow the standard formatting for args/return values? Here's a good example: https://github.com/data-8/datascience/blob/master/datascience/tables.py#L380-L402
E.g.:
...
Args:
table (str): ...
There was a problem hiding this comment.
Also "Table" here can be written as Table or Table - but no parenthesis please.
datascience/tables.py
Outdated
| """ | ||
| Access the i-th row of the given table. | ||
|
|
||
| args: |
There was a problem hiding this comment.
Format comment, same as above
datascience/tables.py
Outdated
| returns: | ||
| Returns a Row object containing the i-th row of the given table | ||
|
|
||
| >>> r.__getitem__(0) |
There was a problem hiding this comment.
To show __getitem__, you can just use the [] notation. Example: r[0] should give you the same result. Change required because it is more Pythonic and easier to read/remember for beginning learners.
datascience/tables.py
Outdated
| Returns the printable representation of the given table as string. | ||
| Uses the standard repr() function. | ||
|
|
||
| >>> r.__repr__() |
There was a problem hiding this comment.
Similar as the last comment, but we should show this as repr(r) instead.
|
Hey! I have updated the docs again. I corrected the incorrect prefix because of which the build was failing. If any other corrections are there, do let me know. |
|
Hi! I think this is awesome!! Style and comments-wise, I think I'm ready to approve. But the build is failing (see above). Can you please take a look and ensure that all tests pass? Once that's done, I think I can approve and merge! Thank you for your hard work on this! |
|
Hi, your changes were still failing the build, so I went ahead and made the required changes to get it to work. Thank you for this work!! |
Have added documentation for the Rows class. Suggestions and corrections are most welcomed.