Ntsika Khanya Gajula#3
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements solutions to a Python formative assessment that includes flight ticket processing functions, a leap year checker, and a reactor status monitoring system with TDD tests.
Key Changes
- Implemented partial solutions for five functions in
system_check.py(ticket processing, leap year calculation, and reactor monitoring) - Added
test_reactor.pywith unit tests for the reactor status function - Included test execution code at module level
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
| test_reactor.py | New test file for reactor_status function with 5 test cases |
| system_check.py | Implements functions for flight ticket parsing, leap year validation, and reactor status monitoring |
| pycache/system_check.cpython-310.pyc | Compiled Python bytecode (generated file) |
Comments suppressed due to low confidence (5)
system_check.py:132
- Test is always false, because of this condition.
elif 1000 < temp > 2000 and radiation > 100:
system_check.py:142
- Print statement may execute during import.
print(is_leap_year(2000))
system_check.py:143
- Print statement may execute during import.
print(is_leap_year(2001))
system_check.py:144
- Print statement may execute during import.
print(is_leap_year(2002))
system_check.py:145
- Print statement may execute during import.
print(is_leap_year(2016))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.assertEqual((15, -5), "Sensor Error") | ||
|
|
||
| def test_critical(self): | ||
| self.assertEqual((2100, 459), "CRITICAL") | ||
|
|
||
| def test_warning(self): | ||
| self.assertEqual((1500, 115), "WARNING") | ||
|
|
||
| def test_maintenance(self): | ||
| self.assertEqual((475, 56), "Maintenance Mode") | ||
|
|
||
| def test_normal_operation(self): | ||
| self.assertEqual((250, 35), "Normal Operation") |
There was a problem hiding this comment.
The assertEqual method is not being called correctly. It should be: self.assertEqual(reactor_status(1500, 115), "WARNING")
| self.assertEqual((15, -5), "Sensor Error") | |
| def test_critical(self): | |
| self.assertEqual((2100, 459), "CRITICAL") | |
| def test_warning(self): | |
| self.assertEqual((1500, 115), "WARNING") | |
| def test_maintenance(self): | |
| self.assertEqual((475, 56), "Maintenance Mode") | |
| def test_normal_operation(self): | |
| self.assertEqual((250, 35), "Normal Operation") | |
| self.assertEqual(reactor_status(15, -5), "Sensor Error") | |
| def test_critical(self): | |
| self.assertEqual(reactor_status(2100, 459), "CRITICAL") | |
| def test_warning(self): | |
| self.assertEqual(reactor_status(1500, 115), "WARNING") | |
| def test_maintenance(self): | |
| self.assertEqual(reactor_status(475, 56), "Maintenance Mode") | |
| def test_normal_operation(self): | |
| self.assertEqual(reactor_status(250, 35), "Normal Operation") |
| self.assertEqual((15, -5), "Sensor Error") | ||
|
|
||
| def test_critical(self): | ||
| self.assertEqual((2100, 459), "CRITICAL") | ||
|
|
||
| def test_warning(self): | ||
| self.assertEqual((1500, 115), "WARNING") | ||
|
|
||
| def test_maintenance(self): | ||
| self.assertEqual((475, 56), "Maintenance Mode") | ||
|
|
||
| def test_normal_operation(self): | ||
| self.assertEqual((250, 35), "Normal Operation") |
There was a problem hiding this comment.
The assertEqual method is not being called correctly. It should be: self.assertEqual(reactor_status(250, 35), "Normal Operation")
| self.assertEqual((15, -5), "Sensor Error") | |
| def test_critical(self): | |
| self.assertEqual((2100, 459), "CRITICAL") | |
| def test_warning(self): | |
| self.assertEqual((1500, 115), "WARNING") | |
| def test_maintenance(self): | |
| self.assertEqual((475, 56), "Maintenance Mode") | |
| def test_normal_operation(self): | |
| self.assertEqual((250, 35), "Normal Operation") | |
| self.assertEqual(reactor_status(15, -5), "Sensor Error") | |
| def test_critical(self): | |
| self.assertEqual(reactor_status(2100, 459), "CRITICAL") | |
| def test_warning(self): | |
| self.assertEqual(reactor_status(1500, 115), "WARNING") | |
| def test_maintenance(self): | |
| self.assertEqual(reactor_status(475, 56), "Maintenance Mode") | |
| def test_normal_operation(self): | |
| self.assertEqual(reactor_status(250, 35), "Normal Operation") |
| print(f"The Flight Number: {fl_num}") | ||
| flight_digits = [] | ||
| for i in fl_num: | ||
| if i.isdigit: | ||
| flight_digits.append(i) | ||
| print(flight_digits) | ||
|
|
There was a problem hiding this comment.
The function should return "Valid - Northbound", "Valid - Southbound", or "Invalid Flight" based on whether the numeric portion of the flight number is even, odd, or invalid. However, the current implementation only extracts and prints the digits without performing the required logic or returning any value.
| print(f"The Flight Number: {fl_num}") | |
| flight_digits = [] | |
| for i in fl_num: | |
| if i.isdigit: | |
| flight_digits.append(i) | |
| print(flight_digits) | |
| # Extract digits from flight number | |
| flight_digits = ''.join([i for i in fl_num if i.isdigit()]) | |
| if not flight_digits: | |
| return "Invalid Flight" | |
| try: | |
| num = int(flight_digits) | |
| except ValueError: | |
| return "Invalid Flight" | |
| if num % 2 == 0: | |
| return "Valid - Northbound" | |
| else: | |
| return "Valid - Southbound" |
| if year % 4 == 0: | ||
| pass | ||
| else: | ||
| return False | ||
| if year % 100 == 0: | ||
| pass | ||
| else: | ||
| return False | ||
| if year % 400 == 0: | ||
| return True | ||
| else: | ||
| return False |
There was a problem hiding this comment.
According to the leap year rules, if the year is divisible by 100, it should proceed to check if it's divisible by 400. However, this implementation returns False when divisible by 100 but not by 400, which is correct, but the logic flow is incorrect. If the year is divisible by 100, it should NOT return True immediately (line 92 is unreachable). The correct logic should check divisibility by 400 and return True/False accordingly, not return True for any year divisible by 100.
| if year % 4 == 0: | |
| pass | |
| else: | |
| return False | |
| if year % 100 == 0: | |
| pass | |
| else: | |
| return False | |
| if year % 400 == 0: | |
| return True | |
| else: | |
| return False | |
| if year % 4 != 0: | |
| return False | |
| if year % 100 != 0: | |
| return True | |
| if year % 400 == 0: | |
| return True | |
| return False |
| return "Sensor Error" | ||
| elif temp > 2000 or radiation > 500: | ||
| return "CRITICAL" | ||
| elif 1000 < temp > 2000 and radiation > 100: |
There was a problem hiding this comment.
The condition 1000 < temp > 2000 is incorrect. This checks if 1000 < temp AND temp > 2000, which can never be true (temp can't be both greater than 1000 and greater than 2000 means less than 2000). The correct condition should be 1000 <= temp <= 2000 and radiation > 100 to check if temp is between 1000 and 2000 (inclusive) as per the requirements.
| elif 1000 < temp > 2000 and radiation > 100: | |
| elif 1000 <= temp <= 2000 and radiation > 100: |
| print(f"The Flight Class: {fl_class}") | ||
|
|
There was a problem hiding this comment.
The function should determine and return the baggage allowance based on the flight class (e.g., "Economy - 20kg", "Business - 40kg", "First Class - 60kg", or "Standard - 0kg"), but it only prints the flight class. The logic to check the class and return the appropriate allowance string is missing.
| print(f"The Flight Class: {fl_class}") | |
| if fl_class.startswith('EC'): | |
| return "Economy - 20kg" | |
| elif fl_class.startswith('BS'): | |
| return "Business - 40kg" | |
| elif fl_class.startswith('FL'): | |
| return "First Class - 60kg" | |
| else: | |
| return "Standard - 0kg" |
| print(f"The Flight Number: {fl_num}") | ||
| flight_digits = [] | ||
| for i in fl_num: | ||
| if i.isdigit: |
There was a problem hiding this comment.
isdigit is a method and must be called with parentheses: i.isdigit(). Without the parentheses, this always evaluates to True (since the method object is truthy), causing all characters to be appended to the list.
| if i.isdigit: | |
| if i.isdigit(): |
| self.assertEqual((15, -5), "Sensor Error") | ||
|
|
||
| def test_critical(self): | ||
| self.assertEqual((2100, 459), "CRITICAL") | ||
|
|
||
| def test_warning(self): | ||
| self.assertEqual((1500, 115), "WARNING") | ||
|
|
||
| def test_maintenance(self): | ||
| self.assertEqual((475, 56), "Maintenance Mode") | ||
|
|
||
| def test_normal_operation(self): | ||
| self.assertEqual((250, 35), "Normal Operation") |
There was a problem hiding this comment.
The assertEqual method expects the actual value as the first argument and the expected value as the second argument. This test is currently calling assertEqual with a tuple as the first argument instead of calling the reactor_status function. It should be: self.assertEqual(reactor_status(15, -5), "Sensor Error")
| self.assertEqual((15, -5), "Sensor Error") | |
| def test_critical(self): | |
| self.assertEqual((2100, 459), "CRITICAL") | |
| def test_warning(self): | |
| self.assertEqual((1500, 115), "WARNING") | |
| def test_maintenance(self): | |
| self.assertEqual((475, 56), "Maintenance Mode") | |
| def test_normal_operation(self): | |
| self.assertEqual((250, 35), "Normal Operation") | |
| self.assertEqual(reactor_status(15, -5), "Sensor Error") | |
| def test_critical(self): | |
| self.assertEqual(reactor_status(2100, 459), "CRITICAL") | |
| def test_warning(self): | |
| self.assertEqual(reactor_status(1500, 115), "WARNING") | |
| def test_maintenance(self): | |
| self.assertEqual(reactor_status(475, 56), "Maintenance Mode") | |
| def test_normal_operation(self): | |
| self.assertEqual(reactor_status(250, 35), "Normal Operation") |
| self.assertEqual((15, -5), "Sensor Error") | ||
|
|
||
| def test_critical(self): | ||
| self.assertEqual((2100, 459), "CRITICAL") | ||
|
|
||
| def test_warning(self): | ||
| self.assertEqual((1500, 115), "WARNING") | ||
|
|
||
| def test_maintenance(self): | ||
| self.assertEqual((475, 56), "Maintenance Mode") | ||
|
|
||
| def test_normal_operation(self): | ||
| self.assertEqual((250, 35), "Normal Operation") |
There was a problem hiding this comment.
The assertEqual method is not being called correctly. It should be: self.assertEqual(reactor_status(2100, 459), "CRITICAL")
| self.assertEqual((15, -5), "Sensor Error") | |
| def test_critical(self): | |
| self.assertEqual((2100, 459), "CRITICAL") | |
| def test_warning(self): | |
| self.assertEqual((1500, 115), "WARNING") | |
| def test_maintenance(self): | |
| self.assertEqual((475, 56), "Maintenance Mode") | |
| def test_normal_operation(self): | |
| self.assertEqual((250, 35), "Normal Operation") | |
| self.assertEqual(reactor_status(15, -5), "Sensor Error") | |
| def test_critical(self): | |
| self.assertEqual(reactor_status(2100, 459), "CRITICAL") | |
| def test_warning(self): | |
| self.assertEqual(reactor_status(1500, 115), "WARNING") | |
| def test_maintenance(self): | |
| self.assertEqual(reactor_status(475, 56), "Maintenance Mode") | |
| def test_normal_operation(self): | |
| self.assertEqual(reactor_status(250, 35), "Normal Operation") |
| @@ -0,0 +1,22 @@ | |||
| import unittest | |||
| from system_check import reactor_status | |||
There was a problem hiding this comment.
Import of 'reactor_status' is not used.
| from system_check import reactor_status |
No description provided.