-
Notifications
You must be signed in to change notification settings - Fork 135
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
Replace Rollback with Scope #444
Conversation
A draft for automatic cm closing: def test_cm_closing(self):
import inspect
this_f = inspect.currentframe()
begin_values = set(id(v) for v in this_f.f_locals.values())
f2 = inspect.getframeinfo(this_f)
q = 5
end_values = set(id(v) for v in this_f.f_locals.values())
ids_to_release = end_values - begin_values
for v in this_f.f_locals.values():
if id(v) in ids_to_release:
if hasattr(v, 'close'):
v.close()
elif hasattr(v, '__exit__'):
v.__exit__() Unfortunately, it looks extremely fragile and restricted. |
@IRDonch, I refactored and simplified the class. |
|
||
def on_error_do(self, callback: Callable, | ||
*args, kwargs: Optional[Dict[str, Any]] = None, | ||
ignore_errors: bool = False): |
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.
ignore_errors: bool = False): | |
ignore_errors: bool = False) -> None: |
(and ditto for the other None
-returning functions)
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.
What is the point in this?
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.
To help typecheckers find errors.
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.
So you suggest to add it to all functions without return value?
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.
I would say yes (if there are annotations on the function's arguments). Otherwise, the return type defaults to Any
.
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.
I'm not convinced it is worthy investment of efforts in most cases. I'm open for a PR on this, but I don't think it is needed for the functions, which names don't not suppose any return value.
Summary
Replaces error rollback utilities with scope utilities. The new class extends the previous situation to be like try-except-finally and allows to enter context managers. This helps to maintain flat code indentation and helps with the objects that require closing / disposal in any cases (i.e. resource managers). The
Project
class is going to be such a manager - this is needed to resolve problems with file removal on Windows.How to test
Checklist
develop
branchLicense
Feel free to contact the maintainers if that's a concern.