Refactor: modularize long methods in Options and checkexpr #19010
Conversation
|
Given that this refactoring touches completely unrelated files, could you please submit these changes separately? |
| @@ -3968,54 +3968,14 @@ def check_op_reversible( | |||
| right_expr: Expression, | |||
| context: Context, | |||
| ) -> tuple[Type, Type]: | |||
| def lookup_operator(op_name: str, base_type: Type) -> Type | None: | |||
There was a problem hiding this comment.
Extracting this helper to method is good and may improve performance of compiled mypy - mypyc isn't doing its best when there are nested functions.
| for name, method, obj, arg in variants: | ||
| with self.msg.filter_errors(save_filtered_errors=True) as local_errors: | ||
| result = self.check_method_call(name, obj, method, [arg], [ARG_POS], context) | ||
| result = self.check_method_call(op_name, obj, method, [arg], [ARG_POS], context) |
There was a problem hiding this comment.
Please wait until #18995 is merged - this will be a conflict (that PR fixes a bug, op_name shouldn't be used here)
There was a problem hiding this comment.
Ough, it is already merged, sorry. Then please don't introduce that bug back!
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
@sterliakov Thanks for the suggestion. I've split the changes as requested. The refactoring of I'll also submit the |
This pull request addresses part of the long-term refactoring goal described in issue #5917, which proposes breaking up very large methods into smaller, more readable components.
Summary of changes
This PR includes two independent refactorings:
Options.__init__: extracted internal logic into_initialize_*methods, grouped by purpose (e.g., caching, build options, warnings).All original comments and default values were preserved for clarity.
check_op_reversible: decomposed into helper methods:_lookup_operator_lookup_definer_determine_operator_order_attempt_operator_applicationsThis makes the logic easier to follow, particularly around Python's operator resolution rules.
Motivation
Notes
Further refactorings aligned with issue #5917 may be submitted in future PRs.