ageron / handson-ml3

A series of Jupyter notebooks that walk you through the fundamentals of Machine Learning and Deep Learning in Python using Scikit-Learn, Keras and TensorFlow 2.
Apache License 2.0
7.93k stars 3.18k forks source link

Consistent use of variable names helps learners #167

Open txoof opened 2 weeks ago

txoof commented 2 weeks ago

Before you create this issue

Is your feature request related to a problem? Please describe. In Chapter 2 - End to end machine learning project, Cells 4, 30 the housing variable is reassigned. It starts off as being raw data, and then turns into the training set. This is confusing and easy to overlook as the reader tries to understand the full pipeline presented in chapter 2.

Describe the solution you'd like Use consistent variable names so we can easily follow the code. In this instance, I forgot somewhere along the line that we reassigned housing to be the test set. When I was trying to understand the full pipeline laid out on pp87-88 (third edition ISBN 978-1-098-12597-4), I kept stumbling over how the raw housing set, created back in cell 4, was going to fit through this pipeline for training. It just didn't make any sense.

I was trying to rebuild the whole pipeline in a group of cells so I could examine each part and it just kept failing because I missed the reassignment in cell 30.

Describe alternatives you've considered Please use consistent variable names throughout. You assigned a great name strat_test_set to the stratified test set. Stick with it through the entire code. It took me about an hour of carefully reading every line in the provided notebook to figure out why this was working.

Additional context Everything works, it's just hard to learn new concepts and keep all the code in your head as you're learning.

ageron commented 1 week ago

Hi @txoof , Thanks for your feedback, I'm sorry for the confusion. You're right that I should not have reassigned housing, that was a bad idea. Note that this variable was reassigned to the training set, not the test set (housing = strat_train_set.copy()). Unfortunately, if I change this now, I'm afraid it will introduce even more confusion for people following along with the book, but I will definitely fix this in the next edition.