Closed swayneh closed 2 years ago
Hey @lindsay-forestell made some changes, please have a look over when you've time.
OK @ateucher @lindsay-forestell I've worked through the changes, branch should be up to date.
@swayneh are you wanting a review of this right now, or are you working on it? I see you left a comment a few days ago but not sure if you are still adding changes.
I think I still have a few comments left, but it's becoming tricky to sort out what has/hasn't been addressed explicitly. If there are no more changes happening locally, I'm happy to just pop in the few suggestions I had myself instead of sorting through the resolved conversations above. I think, based on what I currently see, the last few suggestions include:
@lindsay-forestell thanks for the bullets. Yes, things have gotten a bit complicated over time with the revisions. Here's the latest regarding your bullets above:
-Reconsider keeping apply in the column section (for more complicated functions than * or /) I would add it by way of something like new_column_df.columnA.apply(log) or something right after the / example, and before the assign/lambda example.
_Yes,_I added some code to illustrate apply(): new_cols_df['country_name_chars'] = new_cols_df.country.apply(len)__
-Slightly expand on/call back to dictionaries in the groupby section when using .agg
I added a call out box to explain dictionary.
-Sorting by more than 1 column
I expanded to show how to create for more than one with a list
-Fixing the headers to be the right level
Yes, now everything is at 4.x
-Fix line 447
All references to .columns() are now between 552 and 585
I'm thinking we're ready to merge to main.
It was a bit of a complicated saga getting this up for review, wiht Andy's help ultimately. Also I was getting to the point where I just needed to work on something else for a bit. But its a start.