complex-numbers: check test version and update #1057
Conversation
|
@trangttt thanks for the PR |
| input_number = ComplexNumber(0, 1) | ||
| self.assertEqual(input_number.imaginary, 1) | ||
|
|
||
| def test_maginary_part_of_a_number_with_real_and_imaginary_part(self): |
There was a problem hiding this comment.
Spelling error that wasn't caught in the previous version of this file. Should be
def test_imaginary_part_of_a_number_with_real_and_imaginary_part(self):
| @@ -4,74 +4,64 @@ | |||
|
|
|||
| from complex_numbers import ComplexNumber | |||
|
|
|||
| # Tests adapted from `problem-specifications//canonical-data.json` @ v1.0.0 | |||
There was a problem hiding this comment.
Can you move the comment down by a line for the sake of consistency? (ie. two blank rows before, one after)
| @@ -126,13 +116,37 @@ def test_conjugate_a_purely_imaginary_number(self): | |||
| self.assertEqual(expected.imaginary, | |||
| input_number.conjugate().imaginary) | |||
|
|
|||
| def conjugate_a_number_with_real_and_imaginary_part(self): | |||
| def test_conjugate_a_number_with_real_and_imaginary_part(self): | |||
| input_number = ComplexNumber(1, 1) | |||
| expected = ComplexNumber(1, -1) | |||
| self.assertEqual(expected.real, input_number.conjugate().real) | |||
| self.assertEqual(expected.imaginary, | |||
| input_number.conjugate().imaginary) | |||
There was a problem hiding this comment.
Can you swap the arguments in these asserts? They should follow the convention of assertEqual(actual, expected). This should also be done for the division tests above.
|
@trangttt, are you still working on this? It just needs a few minor changes and then it can be merged. |
|
Sorry for this late update. The email was buried in my mailbox so I didn't notice. Please tell me if I need to update anything. |
| self.assertEqual(first_input.mul(second_input).imaginary, 0) | ||
| second_input = ComplexNumber(3, 4) | ||
| self.assertEqual(first_input.mul(second_input).real, -5) | ||
| self.assertEqual(first_input.mul(second_input).imaginary, 10) | ||
|
|
||
| def test_divide_purely_real_numbers(self): | ||
| input_number = ComplexNumber(1.0, 0.0) |
There was a problem hiding this comment.
Can you swap the order the arguments in these methods to match the convention assertEqual(actual, expected)?
GitHub's displayed code section isn't very helpful here. I'm referring to the division tests (these were already up-to-date, but don't match the conventions).
|
@trangttt, can you swap the argument order in the division tests to match the |
|
Perfect! Thanks, @trangttt! |
* complex-numbers: check test version and update * Swap the order of expected and actual value in assertEqual
* complex-numbers: check test version and update * Swap the order of expected and actual value in assertEqual
#993