Tigers - Lindsey B. #156

Lindseymarcea · 2022-09-23T16:10:54Z

No description provided.

tests for waves 1,2,3 to commit

Lindseymarcea · 2022-09-23T16:17:27Z

My only comment is that after listening to the viewing party intro and reading the instructions I wasn't aware that all of the tests needed to be completed until speaking with Jayce and she informed me, a few days ago that I was. And was not able to complete the full viewing party in time. Test one in wave 2 also passed but did not pass this morning.

chimerror

Good attempt!

Unfortunately, this submission is too incomplete at this point for any other grade besides Red. At the very least, we're expecting waves 1 through 3 to be completed and at least half of wave 4. Wave 5 can be skipped. If those get done, we can move this up to a Yellow. (Obviously if they all get done, we can move to Green!)

In the future if you are facing a deadline and have not completed all waves please reach out to me before submitting an incomplete submission. I would rather grant you an extension to finish it up rather than realize that it's incomplete a week later when I got around to grading it. I could have also verified what you had done and if that would be enough for at least a Yellow. (Admittedly taking so long to get to grading is on me...)

Additionally, there are several very concerning issues and bugs in the code that suggest that you may not quite have had complete understanding of using loops and data structures, which is further reason for me to mark this Red. I've pointed these out in comments.

I also made a few smaller comments about other issues, but I want your main focus to be on getting everything working first.

As am I marking this Red, you will be expected to make another submission to fix the issues. We do have our 1:1 coming up this week, but if you would like to schedule more time to receive help on this, we can schedule that too. I will also be running Catch-up office hours at 1 PM PDT on Tuesday, which is specifically for catching up on older projects, so that might be a good time to get help with this.

I know it's rough to recieve a Red, but I'm hopeful that with some effort and help we can totally get this to Yellow if not Green! You've likely already have had more practice with your work on Adagrams this week, so hopefully some of that helps here!

tests/test_wave_01.py

chimerror · 2022-09-30T23:45:15Z

viewing_party/party.py

@@ -1,23 +1,152 @@
 # ------------- WAVE 1 --------------------

+#from tkinter.tix import InputOnly


Looks like an import statement that ended up not being used. Just a small style thing, it's usually worth removing these before submission to keep the code clean.

Still here after resubmission, but not really something major, so it's fine.

viewing_party/party.py

chimerror · 2022-10-01T00:35:06Z

viewing_party/party.py

+    else: 
+        for i in range(len(user_data["watched"])):
+            genre_list.append((user_data["watched"][i] ["genre"]))
+            i+=1


Like the increment statement above, this increment statement will throw off your loop, causing every other item to be skipped.

This still applies in the updated code, which will definitely cause a bug.

chimerror · 2022-10-01T00:41:35Z

viewing_party/party.py

+
+    for genre in genre_list:
+        count = genre_list.count(genre)
+        genre_dictionary[genre] = count


This works, but there is a small performance hit. Even though we hadn't covered time complexity at the time you wrote this, it will be helpful to understand what's going on.

When calling genre_list.count(genre), it will be an O(n) operation because it will go through every item in genre_list to get the count. However, since we are going through every genre in genre_list in our loop, that is also going to be an O(n) operation causing the entire loop to end up O(n^2).

While count is an incredibly useful method here, we can get this loop down to O(n) by managing genre_dictionary ourselves:

for genre in genre_list: if genre in genre_dictionary.keys(): genre_dictionary[genre] += 1 else genre_dictionary[genre] = 1

While the updated does use genre_dictionary note that we still hit the O(n^2) performance issue that I pointed out last time due to calling genre_list.count(genre) during each iteration of for genre in genre_list. But it's progress, so it's not too concerning.

viewing_party/party.py

… statements and remove skips

…itional code

chimerror

Good Work!

Despite the fact that your wave 5 tests are failing due to a small error, the bug and performance in get_most_watched_genre still in play, and a few other small issues I pointed out, this looks good enough for a Green! Thank you so much for re-submitting, it is much improved and demonstrates good understanding of the material.

chimerror · 2022-10-25T21:28:42Z

tests/test_wave_01.py


-    raise Exception("Test needs to be completed.")
+    # raise Exception("Test needs to be completed.")


As a minor style preference, I recommend just getting rid of commented out code in general unless there's a clear reason to keep it, which should be explained in a comment. This also applies to the comments below where we described how we wanted you to extend the tests, but more just in the sense of keeping code clean.

I'm sure there's a few other cases where this applies, but I'll only point it out once to avoid repeating myself.

chimerror · 2022-10-25T21:29:03Z

tests/test_wave_01.py

@@ -118,13 +114,16 @@ def test_moves_movie_from_watchlist_to_empty_watched():
    # Assert
    assert len(updated_data["watchlist"]) == 0
    assert len(updated_data["watched"]) == 1
+    assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1
+    assert updated_data["watched"][0]["genre"] == GENRE_1
+    assert updated_data["watched"][0]["rating"] == RATING_1


Good thorough test design here!

chimerror · 2022-10-25T21:33:41Z

tests/test_wave_03.py

@@ -55,12 +55,12 @@ def test_friends_unique_movies_not_duplicated():
    # Assert
    assert len(friends_unique_movies) == 3

-    raise Exception("Test needs to be completed.")
+    # raise Exception("Test needs to be completed.")


Not a major deal, but you were expected to expand this test a little beyond just checking the length. The main thing we want to verify is that the duplicated movie (INTRIGUE_3) is not also duplicated in the result, so something like assert friends_unique_movies.count(INTRIGUE_3) == 1 would make sense.

chimerror · 2022-10-25T21:34:44Z

viewing_party/party.py

@@ -1,23 +1,152 @@
 # ------------- WAVE 1 --------------------

+#from tkinter.tix import InputOnly


Still here after resubmission, but not really something major, so it's fine.

chimerror · 2022-10-25T21:41:16Z

viewing_party/party.py


-# -----------------------------------------
+    new_movie = {}


Minor style thing about where you declare variables. Since we only use this new_movie variable in the if title and genre and rating block, we can move its declaration there. In this case it only saves the very minor memory allocation of an empty dictionary, but that's still worth keeping in mind.

In general, I look to delay my variable declarations to the last possible moment, which both avoids unnecessary allocations if the function exits early, and tends to make the variable declaration be as close as possible to where it is used, which means less scrolling if you need to check the declaration to understand what's happening later on.

chimerror · 2022-10-25T21:51:51Z

viewing_party/party.py

+
+    for genre in genre_list:
+        count = genre_list.count(genre)
+        genre_dictionary[genre] = count


While the updated does use genre_dictionary note that we still hit the O(n^2) performance issue that I pointed out last time due to calling genre_list.count(genre) during each iteration of for genre in genre_list. But it's progress, so it's not too concerning.

viewing_party/party.py

chimerror · 2022-10-25T22:30:51Z

viewing_party/party.py

+        for movie in friend["watched"]:
+            if movie not in user_data["watched"] and movie["host"] in user_data["subscriptions"]:
+                recommended.append(movie)
+    return recommended


This works, but has some downsides. Mainly this would have been a good chance to reuse get_friends_unique_watched from wave 4 as a helper function. Which mainly would be nice because it will deduplicate movies unlike the implementation here.

chimerror · 2022-10-25T22:33:11Z

viewing_party/party.py

+
+    favorite_genre = get_most_watched_genre(user_data)
+
+    for friend in user_data["friend"]:


Ah, it looks like the tests for wave 5 are partially failing because this should likely be user_data["friends"] (plural on "friends") rather than user_data["friend"].

Which of course, could have been avoided if you reused get_friends_unique_watched here as a helper function, since that function works.

chimerror · 2022-10-25T22:36:12Z

viewing_party/party.py

+    for movie in user_data["favorites"]:
+        if movie["title"] not in friends_list:
+            recommended_movies.append(movie)
+    return recommended_movies


Despite me mentioning reusing get_friends_unique_watched in other cases, here, get_unique_watched would have likely made more sense... Unless of course we can't guarantee that any movie in user_data["favorites"] is also in user_data["watched"], which is definitely possible but seems unlikely.

If we could assume that every movie in user_data["favorites"] was also in user_data["watched"] Then we would be free to reuse get_unique_watched for slightly cleaner code.

Lindseymarcea added 2 commits September 22, 2022 20:13

Updated README File with name and passed test/

89b8414

tests for waves 1,2,3 to commit

finished

f4466a2

chimerror reviewed Oct 1, 2022

View reviewed changes

Lindseymarcea added 3 commits October 7, 2022 14:07

Passed tests in Wave, 1, 2, and 4 and edited test files to add assert…

bb3a804

… statements and remove skips

Edited Wave 1 code to pass 2 missing tests and modified tests and add…

61a8333

…itional code

Unsaved changes added

521d978

chimerror reviewed Oct 25, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tigers - Lindsey B. #156

Tigers - Lindsey B. #156

Lindseymarcea commented Sep 23, 2022

Lindseymarcea commented Sep 23, 2022

chimerror left a comment

chimerror Sep 30, 2022

chimerror Oct 25, 2022

chimerror Oct 1, 2022

chimerror Oct 25, 2022

chimerror Oct 1, 2022

chimerror Oct 25, 2022

chimerror left a comment

chimerror Oct 25, 2022

chimerror Oct 25, 2022

chimerror Oct 25, 2022

chimerror Oct 25, 2022

chimerror Oct 25, 2022

chimerror Oct 25, 2022

chimerror Oct 25, 2022

chimerror Oct 25, 2022

chimerror Oct 25, 2022

		@@ -1,23 +1,152 @@
		# ------------- WAVE 1 --------------------

		#from tkinter.tix import InputOnly


		raise Exception("Test needs to be completed.")
		# raise Exception("Test needs to be completed.")


		favorite_genre = get_most_watched_genre(user_data)

		for friend in user_data["friend"]:

Tigers - Lindsey B. #156

Are you sure you want to change the base?

Tigers - Lindsey B. #156

Conversation

Lindseymarcea commented Sep 23, 2022

Lindseymarcea commented Sep 23, 2022

chimerror left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chimerror left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment