Skip to content

Conversation

@jorisvandenbossche
Copy link
Collaborator

No description provided.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

@@ -0,0 +1,684 @@
{
Copy link
Collaborator Author

@jorisvandenbossche jorisvandenbossche Sep 21, 2019

Choose a reason for hiding this comment

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

functionalality -> functionality

There is also something wrong in the sentence " the reading functionalality provide the read_csv function to read the data into .."


Reply via ReviewNB

Copy link
Owner

Choose a reason for hiding this comment

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

ok.

@@ -0,0 +1,684 @@
{
Copy link
Collaborator Author

@jorisvandenbossche jorisvandenbossche Sep 21, 2019

Choose a reason for hiding this comment

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

Now that the default repr only shows the first 5 and last 5 rows, you actually get head and tail in one go ..

So wondering if we should just show that first, still saying afterwards (or showing one of the two) that you can use those for more specifically seeing first or last n rows.


Reply via ReviewNB

Copy link
Owner

Choose a reason for hiding this comment

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

Adjusted as such: using default repr first, introduce head and add tail as a note.

@@ -0,0 +1,684 @@
{
Copy link
Collaborator Author

@jorisvandenbossche jorisvandenbossche Sep 21, 2019

Choose a reason for hiding this comment

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

Is the index=False strictly needed?

If we don't explain it, maybe rather not use it?


Reply via ReviewNB

Copy link
Owner

Choose a reason for hiding this comment

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

I would keep it in there, as otherwise the next read_excel output has the Unnamed: 0 column which is inconvenient. I explained the reasoning of the additional parameter.

@@ -0,0 +1,684 @@
{
Copy link
Collaborator Author

@jorisvandenbossche jorisvandenbossche Sep 21, 2019

Choose a reason for hiding this comment

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

to_* functions -> to_* methods ?

(not sure we already explained that difference (might come later?))


Reply via ReviewNB

@@ -0,0 +1,684 @@
{
Copy link
Collaborator Author

@jorisvandenbossche jorisvandenbossche Sep 21, 2019

Choose a reason for hiding this comment

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

An answer needs to be added here?


Reply via ReviewNB

Copy link
Owner

Choose a reason for hiding this comment

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

First idea was to explain the [TAB] idea to explore the read_* namespace but as we can't really take any assumptions on the IDE of the user, I excluded this.

@jorisvandenbossche
Copy link
Collaborator Author

In this notebook, I would say something more explicitly about pandas "supporting many different file formats or data sources out of the box" (csv, excel, sql, json, parquet, ...), as that is one of the strengths of pandas I think.

@@ -0,0 +1,684 @@
{
Copy link

@justmarkham justmarkham Sep 27, 2019

Choose a reason for hiding this comment

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

I think it's worth mentioning explicitly that there are no parentheses after dtypes because it's an attribute and not a method.


Reply via ReviewNB

Copy link
Owner

@stijnvanhoey stijnvanhoey Oct 7, 2019

Choose a reason for hiding this comment

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

Good point, added this as a note to the tutorial.

Copy link
Owner

I added the comment on the multiple formats more explicitly. Moreover, the figure supports this statement as well.

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.

4 participants