-
-
Notifications
You must be signed in to change notification settings - Fork 154
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
better rect tests using literal values #3190
base: main
Are you sure you want to change the base?
Conversation
Hi antoine, since you're new to contributing, let me give you one little advice on how you can improve your PRs next time. You don't have to mention the link of the issue in the title, you can mention it in your message directly. With this, you can opt for a more descriptive title for your PR. |
I think there is a little quiproquo, when I talked about tests for literal values, I meant for rect attributes, not these ones. |
Hello @bilhox, I think I don't really understand what should be done. Could you explain more specifically, maybe ? |
self.assertEqual((5, 8), r2.center) | ||
self.assertEqual(4, r2.left) | ||
self.assertEqual(7, r2.top) | ||
self.assertEqual(6, r2.right) | ||
self.assertEqual(9, r2.bottom) | ||
self.assertEqual(2, r2.width) | ||
self.assertEqual(2, r2.height) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some whitespace snuck by - that's why some tests are failing. I recommend using autoformatting tools to fix that.
You can either:
pip install pre-commit
pre-commit run -a
pre-commit install # completely optional, will cause the pre-commit to run every time you make a commit.
Or
python dev.py format
Both I recommend running in a virtualenv.
What I meant is we should have a test similar to |
Fix the following functions in rect_test to use literal values for attribute checks : test_inflate__larger, test_inflate__smaller, test_inflate_ip__larger and test_inflate_ip__smaller.