Skip to content
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

Argument Clinic: split out global stateless helpers and constants from clinic.py #113317

Closed
erlend-aasland opened this issue Dec 20, 2023 · 8 comments · Fixed by #117513
Closed
Labels
topic-argument-clinic type-feature A feature request or enhancement

Comments

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Dec 20, 2023

Feature or enhancement

clinic.py has a lot of globals in various forms. Getting rid of the globals will make it possible to split out clinic.py in multiple files (#113299). Combined, this will impact readability and maintainability of Argument Clinic in a positive way.

Most globals can be easily dealt with. We can structure them into two groups:

  • global constants that are not mutated
  • stateless helper functions and classes

It can make sense split out some of these in separate files (for example a libclinic package, as suggested in #113309). For some globals, it makes more sense to embed them into one of the existing classes (for example the global version variable clearly belongs in DSLParser).

Suggesting to start with the following:

  • move stateless text accumulator helpers into a Tools/clinic/libclinic/accumulators.py file
  • move stateless text formatting helpers into a Tools/clinic/libclinic/formatters.py file
  • move version into DSLParser and version helper into a Tools/clinic/libclinic/version.py file

Linked PRs

@erlend-aasland erlend-aasland added type-feature A feature request or enhancement topic-argument-clinic labels Dec 20, 2023
@erlend-aasland
Copy link
Contributor Author

Proof-of-concept branch for version: Argument-Clinic#28

@erlend-aasland
Copy link
Contributor Author

In #113270 (comment), the need for a refactoring of the docstring generation code arose. If we split out formatting helpers and text accumulators into a libclinic, we can also split out the docstring rendering (format_docstring()).

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Dec 21, 2023

Follow-up item: when libclinic has been established, we can start partitioning the test suite (Lib/test/test_clinic.py) as well.

@erlend-aasland erlend-aasland changed the title Argument Clinic: split out stateless helpers and constants from clinic.py Argument Clinic: split out global stateless helpers and constants from clinic.py Dec 21, 2023
@erlend-aasland
Copy link
Contributor Author

FTR, the global version variable was removed with PR #113341.

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Dec 22, 2023

Moving the text accumulator APIs (TextAccumulator, _TextAccumulator, text_accumulator, and _text_accumulator) into a libclinic would imply exposing underscore prefixed methods in __all__; that feels weird. Suggested first steps:

I did some experimenting in Argument-Clinic#30.

erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 22, 2023
…APIs

Replace the internal accumulator APIs by using lists of strings and join().

Yak-shaving for separating out formatting code into a separate file.
erlend-aasland added a commit that referenced this issue Dec 22, 2023
…113402)

Replace the internal accumulator APIs by using lists of strings and join().

Yak-shaving for separating out formatting code into a separate file.

Co-authored-by: Alex Waygood <[email protected]>
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 22, 2023
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 22, 2023
Split up clinic.py by establishing libclinic as a support package for
Argument Clinic. Get rid of clinic.py globals by either making them
class members, or by putting them into libclinic.

- Move INCLUDE_COMMENT_COLUMN to BlockPrinter
- Move NO_VARARG to CLanguage
- Move formatting helpers to libclinic
- Move some constants to libclinic (and annotate them as Final)
erlend-aasland added a commit that referenced this issue Dec 23, 2023
Split up clinic.py by establishing libclinic as a support package for
Argument Clinic. Get rid of clinic.py globals by either making them
class members, or by putting them into libclinic.

- Move INCLUDE_COMMENT_COLUMN to BlockPrinter
- Move NO_VARARG to CLanguage
- Move formatting helpers to libclinic
- Move some constants to libclinic (and annotate them as Final)
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 23, 2023
- Move strip_leading_and_trailing_blank_lines() and make it internal to libclinic
- Move normalize_snippet() to libclinic
- Move format_escape() to libclinic
- Move wrap_declarations() to libclinic
erlend-aasland added a commit that referenced this issue Dec 23, 2023
Move the following global helpers into libclinic:
- format_escape()
- normalize_snippet()
- wrap_declarations()

Also move strip_leading_and_trailing_blank_lines() and make it internal to libclinic.
ryan-duve pushed a commit to ryan-duve/cpython that referenced this issue Dec 26, 2023
…APIs (python#113402)

Replace the internal accumulator APIs by using lists of strings and join().

Yak-shaving for separating out formatting code into a separate file.

Co-authored-by: Alex Waygood <[email protected]>
ryan-duve pushed a commit to ryan-duve/cpython that referenced this issue Dec 26, 2023
…3414)

Split up clinic.py by establishing libclinic as a support package for
Argument Clinic. Get rid of clinic.py globals by either making them
class members, or by putting them into libclinic.

- Move INCLUDE_COMMENT_COLUMN to BlockPrinter
- Move NO_VARARG to CLanguage
- Move formatting helpers to libclinic
- Move some constants to libclinic (and annotate them as Final)
ryan-duve pushed a commit to ryan-duve/cpython that referenced this issue Dec 26, 2023
…113438)

Move the following global helpers into libclinic:
- format_escape()
- normalize_snippet()
- wrap_declarations()

Also move strip_leading_and_trailing_blank_lines() and make it internal to libclinic.
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Dec 27, 2023

We should put some thought into how we structure the rest of libclinic. Victor hashed out a proof-of-concept libclinic in #113309 #113300. We can use that PR as inspiration. We might want to do some more yak-shaving before we make such a grand transition:

  • rework the error handling so warn and fail no longer depend on global variables
  • refactor global configuration to be part of the application (converters, extensions/languages, destinations?, etc.), instead of hard-coded in the various classes (and as globals)

erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 27, 2023
Rework error handling in the C preprocessor helper. Instead of monkey-
patching the cpp.Monitor.fail() method from within clinic.py, rewrite
cpp.py to use a subclass of the ClinicError exception.

Yak-shaving in preparation for putting cpp.py into libclinic.
erlend-aasland added a commit that referenced this issue Dec 27, 2023
Rework error handling in the C preprocessor helper. Instead of monkey-
patching the cpp.Monitor.fail() method from within clinic.py, rewrite
cpp.py to use a subclass of the ClinicError exception. As a side-effect,
ClinicError is moved into Tools/clinic/libclinic/errors.py.

Yak-shaving in preparation for putting cpp.py into libclinic.
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Jan 12, 2024
Establish Tools/clinic/libclinic/utils.py and move the following
functions over there:

- compute_checksum()
- create_regex()
- write_file()
vstinner added a commit that referenced this issue Apr 11, 2024
* Move ifndef_symbols, includes and add_include() from Clinic to
  Codegen. Add a 'codegen' (Codegen) attribute to Clinic.
* Remove libclinic.crenderdata module: move code to libclinic.codegen.
* BlockPrinter.print_block(): remove unused 'limited_capi' argument.
  Remove also 'core_includes' parameter.
* Add get_includes() methods.
* Make Codegen.ifndef_symbols private.
* Make Codegen.includes private.
* Make CConverter.includes private.
vstinner added a commit to vstinner/cpython that referenced this issue Apr 11, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
Move Module, Class, Function and Parameter classes to a new
libclinic.function module.

Move VersionTuple and Sentinels to libclinic.utils.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
* Move Block and BlockParser classes to a new libclinic.block_parser
  module.
* Move Language and PythonLanguage classes to a new
  libclinic.language module.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
* Move CConverter class to a new libclinic.converter module.
* Move CRenderData and Include classes to a new libclinic.crenderdata
  module.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…116853)

* Add a new create_parser_namespace() function for
  PythonParser to pass objects to executed code.
* In run_clinic(), list converters using 'converters' and
  'return_converters' dictionarties.
* test_clinic: add 'object()' return converter.
* Use also create_parser_namespace() in eval_ast_expr().

Co-authored-by: Erlend E. Aasland <[email protected]>
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…thon#117315)

Move the following converter classes to libclinic.converters:

* PyByteArrayObject_converter
* PyBytesObject_converter
* Py_UNICODE_converter
* Py_buffer_converter
* Py_complex_converter
* Py_ssize_t_converter
* bool_converter
* byte_converter
* char_converter
* defining_class_converter
* double_converter
* fildes_converter
* float_converter
* int_converter
* long_converter
* long_long_converter
* object_converter
* self_converter
* short_converter
* size_t_converter
* slice_index_converter
* str_converter
* unicode_converter
* unsigned_char_converter
* unsigned_int_converter
* unsigned_long_converter
* unsigned_long_long_converter
* unsigned_short_converter

Move also the following classes to libclinic.converters:

* buffer
* robuffer
* rwbuffer

Move the following functions to libclinic.converters:

* correct_name_for_self()
* r()
* str_converter_key()

Move Null and NULL to libclinic.utils.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…thon#117451)

Move the following converter classes to libclinic.return_converters:

* CReturnConverter
* CReturnConverterAutoRegister
* Py_ssize_t_return_converter
* bool_return_converter
* double_return_converter
* float_return_converter
* int_return_converter
* long_return_converter
* size_t_return_converter
* unsigned_int_return_converter
* unsigned_long_return_converter

Move also the add_c_return_converter() function there.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
)

Add libclinic.clanguage module and move the following classes and
functions there:

* CLanguage
* declare_parser()

Add libclinic.codegen and move the following classes there:

* BlockPrinter
* BufferSeries
* Destination

Move the following functions to libclinic.function:

* permute_left_option_groups()
* permute_optional_groups()
* permute_right_option_groups()
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…hon#117513)

Add libclinic.parser module and move the following classes and
functions there:

* Parser
* PythonParser
* create_parser_namespace()

Add libclinic.dsl_parser module and move the following classes,
functions and variables there:

* ConverterArgs
* DSLParser
* FunctionNames
* IndentStack
* ParamState
* StateKeeper
* eval_ast_expr()
* unsupported_special_methods

Add libclinic.app module and move the Clinic class there.

Add libclinic.cli module and move the following functions there:

* create_cli()
* main()
* parse_file()
* run_clinic()
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
* Move ifndef_symbols, includes and add_include() from Clinic to
  Codegen. Add a 'codegen' (Codegen) attribute to Clinic.
* Remove libclinic.crenderdata module: move code to libclinic.codegen.
* BlockPrinter.print_block(): remove unused 'limited_capi' argument.
  Remove also 'core_includes' parameter.
* Add get_includes() methods.
* Make Codegen.ifndef_symbols private.
* Make Codegen.includes private.
* Make CConverter.includes private.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
@vstinner
Copy link
Member

vstinner commented Jun 2, 2024

@erlend-aasland: Can we close this issue now?

@erlend-aasland
Copy link
Contributor Author

Definitely!

Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…APIs (python#113402)

Replace the internal accumulator APIs by using lists of strings and join().

Yak-shaving for separating out formatting code into a separate file.

Co-authored-by: Alex Waygood <[email protected]>
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…3414)

Split up clinic.py by establishing libclinic as a support package for
Argument Clinic. Get rid of clinic.py globals by either making them
class members, or by putting them into libclinic.

- Move INCLUDE_COMMENT_COLUMN to BlockPrinter
- Move NO_VARARG to CLanguage
- Move formatting helpers to libclinic
- Move some constants to libclinic (and annotate them as Final)
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…113438)

Move the following global helpers into libclinic:
- format_escape()
- normalize_snippet()
- wrap_declarations()

Also move strip_leading_and_trailing_blank_lines() and make it internal to libclinic.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…#113525)

Rework error handling in the C preprocessor helper. Instead of monkey-
patching the cpp.Monitor.fail() method from within clinic.py, rewrite
cpp.py to use a subclass of the ClinicError exception. As a side-effect,
ClinicError is moved into Tools/clinic/libclinic/errors.py.

Yak-shaving in preparation for putting cpp.py into libclinic.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…#113986)

Establish Tools/clinic/libclinic/utils.py and move the following
functions over there:

- compute_checksum()
- create_regex()
- write_file()
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…ython#114330)

Make it possible for a converter to have multiple includes, by collecting
them in a list on the converter instance. This implies converter includes
are added during template generation, so we have to add them to the
clinic instance at the end of the template generation instead of in the
beginning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-argument-clinic type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants