gh-104683: Argument clinic: pass `clinic` as a parameter where possible #107435

AlexWaygood · 2023-07-29T16:41:59Z

This PR started off life with me wanting to get rid of this at the end of Clinic.__init__, which feels like an awful hack to me:

cpython/Tools/clinic/clinic.py

Lines 2206 to 2207 in d0dcd27

    
           global clinic 
        
           clinic = self

clinic is elsewhere set to None in the global namespace in two places:

cpython/Tools/clinic/clinic.py

Line 2085 in d0dcd27

clinic = None

cpython/Tools/clinic/clinic.py

Line 5621 in d0dcd27

clinic = None

Unfortunately, removing this hack completely is rather difficult, due to the fact that warn_or_fail relies on clinic being a global variable! Passing clinic as a parameter to warn_or_fail would mean that every usage of warn() or fail() would have to be updated, which would have led to a huge diff.

cpython/Tools/clinic/clinic.py

Lines 155 to 171 in d0dcd27

    
           def warn_or_fail( 
        
               *args: object, 
        
               fail: bool = False, 
        
               filename: str | None = None, 
        
               line_number: int | None = None, 
        
           ) -> None: 
        
               joined = " ".join([str(a) for a in args]) 
        
               add, output = text_accumulator() 
        
               if fail: 
        
                   add("Error") 
        
               else: 
        
                   add("Warning") 
        
               if clinic: 
        
                   if filename is None: 
        
                       filename = clinic.filename 
        
                   if getattr(clinic, 'block_parser', None) and (line_number is None): 
        
                       line_number = clinic.block_parser.line_number

Nonetheless, this PR still feels like a (small) improvement over the current state of affairs. In particular, by passing clinic as a parameter to output_templates, we remove the need for an assert clinic is not None assertion. This PR also reduces the changes that will be requir 8000 ed in the future, should we want to move output_templates into a separate module...

Issue: Modernise code in Tools/clinic/ #104683

erlend-aasland

Makes sense. Thanks!

Argument clinic: pass clinic as a parameter where possible

b659498

AlexWaygood added the skip news label Jul 29, 2023

AlexWaygood requested a review from erlend-aasland as a code owner July 29, 2023 16:42

bedevere-bot mentioned this pull request Jul 29, 2023

Modernise code in Tools/clinic/ #104683

Closed

7 tasks

bedevere-bot added the awaiting core review label Jul 29, 2023

erlend-aasland approved these changes Jul 29, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Jul 29, 2023

AlexWaygood merged commit 5113ed7 into python:main Jul 29, 2023

bedevere-bot removed the awaiting merge label Jul 29, 2023

AlexWaygood deleted the clinic-global-usage branch July 29, 2023 18:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-104683: Argument clinic: pass `clinic` as a parameter where possible #107435

gh-104683: Argument clinic: pass `clinic` as a parameter where possible #107435

	def warn_or_fail(
	*args: object,
	fail: bool = False,
	filename: str \| None = None,
	line_number: int \| None = None,
	) -> None:
	joined = " ".join([str(a) for a in args])
	add, output = text_accumulator()
	if fail:
	add("Error")
	else:
	add("Warning")
	if clinic:
	if filename is None:
	filename = clinic.filename
	if getattr(clinic, 'block_parser', None) and (line_number is None):
	line_number = clinic.block_parser.line_number

gh-104683: Argument clinic: pass clinic as a parameter where possible #107435

gh-104683: Argument clinic: pass clinic as a parameter where possible #107435

Conversation

Choose a reason for hiding this comment

gh-104683: Argument clinic: pass `clinic` as a parameter where possible #107435

gh-104683: Argument clinic: pass `clinic` as a parameter where possible #107435