Improved the behavior of the contract checking new_setattr function #929
Conversation
…ve-behaviour-new_setattr
There was a problem hiding this comment.
@Bruce-8 good work. Revert the changes you made to the ENABLE_CONTRACT_CHECKING constant and keep everything else and I'll take another look. You can (afterwards) do that part in a separate PR.
Pull Request Test Coverage Report for Build 5569447533
💛 - Coveralls |
CHANGELOG.md
Outdated
| @@ -26,6 +26,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | |||
| - When running `check_contracts` on a class with type aliases as type annotations for its attributes, the `NameError` | |||
| that appears (which indicates that the type alias is undefined) is now resolved. | |||
| - The default value of `pyta-number-of-messages` is now 0. This automatically displays all occurrences of the same error. | |||
| - For the contract checking `new_setattr` function, the check for `ENABLE_CONTRACT_CHECKING` is now at the top of the | |||
There was a problem hiding this comment.
Update this to remove the reference to ENABLE_CONTRACT_CHECKING. Also you don't need to go into details about the efficiency improvements: reword to just say "improve efficiency" in the changelog entry.
python_ta/contracts/__init__.py
Outdated
| if klass_mod is not None and ENABLE_CONTRACT_CHECKING: | ||
| try: | ||
| _check_invariants(self, klass, klass_mod.__dict__) | ||
| except PyTAContractError as e: | ||
| if original_attr_value is None: |
There was a problem hiding this comment.
This check doesn't work if the original attribute value was None, which is a different case from the original attribute note existing at all. You can use a separate variable to track whether there was an original attribute. And make sure to add a test for this case as well.
| from python_ta.contracts import check_contracts | ||
|
|
||
|
|
||
| def test_class_attr_value_restores_to_original_if_violates_rep_inv() -> None: |
There was a problem hiding this comment.
Always make sure to add a docstring describing the purpose of each test case.
Motivation and Context
For the
new_setattrfunction inpython_ta/contracts/__init__.py, this pull request re-organizes certain variable definitions, changes the use of certaininspectfunctions, and fixes a bug with attribute value restoration when representation invariants are violated.Your Changes
Description:
klass, but notself/name/value, to be defined in the outer function rather than insidenew_setattr.inspect.getouterframeswith thef_backto improve efficiency of code._check_invariantscall raises an error.Type of change (select all that apply):
Testing
f_backrepresents the frame that's currently being checked in the code._check_invariantscall raises an error._check_invariantscall raises an error.Questions and Comments (if applicable)
For the 10 tests that didn't pass in
test_class_contracts, I believe it was due to my change of movingENABLE_CONTRACT_CHECKINGto the top of the function body, so I don't think there's a way to make this test pass unless I don't don't move the constant. Additionally, for the 6 tests that didn't pass intest_contracts, the tests all passed in PyCharm when I ran them on my own branch, and I'm not too sure why that's the case because I double checked that the tests were the same both here and on my branch.Checklist