Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor laundry.py to use NamedTuple, add tests #189

Merged
merged 5 commits into from
Aug 24, 2024
Merged

Conversation

tianyizheng02
Copy link
Contributor

@tianyizheng02 tianyizheng02 commented Aug 8, 2024

Refactor laundry.py to store API output using NamedTuples rather than plain Python dicts. This makes the output more user-friendly, less error-prone, and easier to type hint. This refactoring also modifies the API behavior as follows:

  1. JSON objects that don't qualify as a known laundry machine type (washer, dryer, or combo) will now be skipped, whereas previously they would be marked as "unknown". These objects should be skipped because they may represent things that aren't laundry machines, such as card readers.
  2. The time_remaining field for laundry machines can now be None if the laundry machine is unavailable (either out of service or offline).
  3. The data for each machine now includes a name field that stores the user-facing name of the machine (A1, B3, G10, etc).

Add unit tests for laundry.py to increase test coverage to 95%. The new tests use mocked data for Towers laundry machines, the vast majority of which are combo machines rather than standalone washers or dryers.

Fix a heretofore unknown bug that was caught during the aforementioned unit testing: "washNdry" was misspelled as "washNDry", which was causing the API to mark all combo machines as "unknown". This typo hadn't been caught because there had been no unit tests that included data for combo machines.

Refactor laundry.py to store API output using NamedTuples rather than
plain Python dicts. This makes the output more user-friendly, less
error-prone, and easier to type hint. This refactoring also modifies the
API behavior as follows:
1. JSON objects that don't qualify as a known laundry machine type
   (washer, dryer, or combo) will now be skipped, whereas previously
   they would be marked as "unknown". These objects should be skipped
   because they may represent things that aren't laundry machines, such
   as card readers.
2. The time_remaining field for laundry machines can now be None if the
   laundry machine is unavailable (either out of service or offline).

Add unit tests for laundry.py to increase test coverage to 100%. The new
tests use mocked data for Towers laundry machines, the vast majority of
which are combo machines rather than standalone washers or dryers.

Fix a heretofore unknown bug that was caught during the aforementioned
unit testing: "washNdry" was misspelled as "washNDry", which was causing
the API to mark all combo machines as "unknown". This typo hadn't been
caught because there had been no unit tests that included data for combo
machines.
Copy link
Member

@RitwikGupta RitwikGupta left a comment

Choose a reason for hiding this comment

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

The TODOs need to be addressed before merging this PR. Mark this as WIP in the meantime.

pittapi/laundry.py Show resolved Hide resolved
pittapi/laundry.py Show resolved Hide resolved
pittapi/laundry.py Outdated Show resolved Hide resolved
@tianyizheng02 tianyizheng02 marked this pull request as draft August 11, 2024 03:13
@tianyizheng02

This comment has been minimized.

Replace mock JSON test data for Sutherland East with new mock JSON test
data for Holland. Because this data was collected close to the start of
the fall semester, when the laundry machines are actually being used, it
contains instances of status_toggle = 1, which clears up the TODO of
what that status value represented. The meaning of this value has now
been documented.

Rewrite the logic for parsing JSON objects for laundry machines. The
logic now accurately parses combo machines and adds support for double
machines, which are two washers or two dryers in a single machine. This
resolves the two remaining TODOs about combo machines.
@tianyizheng02 tianyizheng02 marked this pull request as ready for review August 24, 2024 20:48
@tianyizheng02
Copy link
Contributor Author

@RitwikGupta I've addressed the TODOs. I was able to figure out the TODO for status_toggle = 1 using laundry data from Holland that had the missing status value. I was able to figure out the TODOs for combo machines based on @jkruse7's observation in #135 about even-/odd-numbered machines.

@RitwikGupta
Copy link
Member

Excellent work @tianyizheng02 and @jkruse7!

@tianyizheng02 tianyizheng02 merged commit 275fb92 into dev Aug 24, 2024
4 checks passed
@tianyizheng02 tianyizheng02 deleted the refactor-laundry branch August 24, 2024 23:52
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.

2 participants