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

Modernise code in Tools/clinic/ #104683

Closed
5 of 7 tasks
AlexWaygood opened this issue May 20, 2023 · 5 comments
Closed
5 of 7 tasks

Modernise code in Tools/clinic/ #104683

AlexWaygood opened this issue May 20, 2023 · 5 comments
Labels
topic-argument-clinic type-feature A feature request or enhancement

Comments

@AlexWaygood
Copy link
Member

AlexWaygood commented May 20, 2023

Feature or enhancement

Modernise some Python anachronisms in Tools/clinic/, to make the code more readable and maintainable. Some things that can be easily done:

Cc. @erlend-aasland

Linked PRs

@AlexWaygood AlexWaygood added type-feature A feature request or enhancement topic-argument-clinic labels May 20, 2023
AlexWaygood added a commit that referenced this issue May 20, 2023
For code readability. Instances of `builtins.dict` have been ordered since 3.6, and have been guaranteed by the language to be ordered since Python 3.7. Argument Clinic now requires Python 3.10+.
AlexWaygood added a commit that referenced this issue May 20, 2023
…movesuffix` (#104685)

Both methods were new in Python 3.9.
@erlend-aasland
Copy link
Contributor

erlend-aasland commented May 20, 2023

⚠️ Refactoring is a useful tool, but it is also risky business. We must make sure we've got good test coverage for all touched code. If we're touching uncovered branches, we must add new tests in a separate PR first ⚠️

erlend-aasland added a commit that referenced this issue May 20, 2023
- Make some string interpolations more readable using f-strings or
  explicit parametrisation
- Remove unneeded open() mode specifiers

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
AlexWaygood added a commit to AlexWaygood/cpython that referenced this issue May 21, 2023
@erlend-aasland
Copy link
Contributor

The parameter_state state machine could be made slightly more readable by using enums, for example:

cpython/Tools/clinic/clinic.py

Lines 4711 to 4726 in 35963da

# These rules are enforced with a single state variable:
# "parameter_state". (Previously the code was a miasma of ifs and
# separate boolean state variables.) The states are:
#
# [ [ a, b, ] c, ] d, e, f=3, [ g, h, [ i ] ] <- line
# 01 2 3 4 5 6 <- state transitions
#
# 0: ps_start. before we've seen anything. legal transitions are to 1 or 3.
# 1: ps_left_square_before. left square brackets before required parameters.
# 2: ps_group_before. in a group, before required parameters.
# 3: ps_required. required parameters, positional-or-keyword or positional-only
# (we don't know yet). (renumber left groups!)
# 4: ps_optional. positional-or-keyword or positional-only parameters that
# now must have default values.
# 5: ps_group_after. in a group, after required parameters.
# 6: ps_right_square_after. right square brackets after required parameters.

erlend-aasland added a commit that referenced this issue Jul 3, 2023
Use enums and pattern matching to make the code more readable.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Jul 5, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 5, 2023
pythonGH-106443)

(cherry picked from commit a941bd6)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 5, 2023
pythonGH-106443)

(cherry picked from commit a941bd6)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
AlexWaygood pushed a commit that referenced this issue Jul 5, 2023
….c (GH-106443) (#106444)

gh-104683: Rename Lib/test/clinic.test as Lib/test/clinic.test.c (GH-106443)
(cherry picked from commit a941bd6)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
AlexWaygood pushed a commit that referenced this issue Jul 5, 2023
….c (GH-106443) (#106445)

gh-104683: Rename Lib/test/clinic.test as Lib/test/clinic.test.c (GH-106443)
(cherry picked from commit a941bd6)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
carljm added a commit to carljm/cpython that referenced this issue Jul 5, 2023
* main: (39 commits)
  pythongh-102542 Remove unused bytes object and bytes slicing (python#106433)
  Clarify state of CancelledError in doc (python#106453)
  pythongh-64595: Fix regression in file write logic in Argument Clinic (python#106449)
  pythongh-104683: Rename Lib/test/clinic.test as Lib/test/clinic.test.c (python#106443)
  tp_flags docs: fix indentation (python#106420)
  pythongh-104050: Partially annotate Argument Clinic CLanguage class (python#106437)
  pythongh-106368: Add tests for formatting helpers in Argument Clinic (python#106415)
  pythongh-104050: Annotate Argument Clinic parameter permutation helpers (python#106431)
  pythongh-104050: Annotate toplevel functions in clinic.py (python#106435)
  pythongh-106320: Fix specialize.c compilation by including pycore_pylifecycle.h (python#106434)
  Add some codeowners for `Tools/clinic/` (python#106430)
  pythongh-106217: Truncate the issue body size of `new-bugs-announce-notifier` (python#106423)
  pythongh-61215: Rename `wait_until_any_call` to `wait_until_any_call_with` (python#106414)
  pythongh-106162: array: suppress warning in test_array (python#106404)
  pythongh-106320: Remove _PyInterpreterState_HasFeature() (python#106425)
  pythonGH-106360: Support very basic superblock introspection (python#106422)
  pythongh-106406: Fix _Py_IsInterpreterFinalizing() in _winapi.c (python#106408)
  pythongh-106396: Special-case empty format spec to gen empty JoinedStr node (python#106401)
  pythongh-106368: Add tests for permutation helpers in Argument Clinic (python#106407)
  pythonGH-106008: Fix refleak when peepholing `None` comparisons (python#106367)
  ...
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Aug 4, 2023
erlend-aasland added a commit that referenced this issue Aug 4, 2023
…#107638)

Also make it a cached property.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
AlexWaygood added a commit that referenced this issue Aug 5, 2023
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Aug 8, 2023
Explicitly exclude Lib/test/clinic.test.c when running make clinic.
erlend-aasland added a commit that referenced this issue Aug 8, 2023
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
erlend-aasland added a commit that referenced this issue Aug 8, 2023
Extract helper methods for formatting the signature and parameter
sections, and clean up the remaining function body.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
erlend-aasland added a commit that referenced this issue Aug 9, 2023
…107790)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Aug 15, 2023
Reduce duplicate code in state_modulename_name()
erlend-aasland added a commit that referenced this issue Aug 16, 2023
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Aug 17, 2023
…e_and_class()

The hasattr(parent, "classes") check is unneeded, since 'parent' is an
instance of either the Module, Class, or Clinic classes, and all of
them has a "classes" attribute.
erlend-aasland added a commit that referenced this issue Aug 17, 2023
…class() (#108092)

'not hasattr(parent, "classes")' is always false, since 'parent' is an
instance of either the Module, Class, or Clinic classes, and all of
them has a "classes" attribute.
@erlend-aasland
Copy link
Contributor

Argument Clinic is now slightly more readable, thanks to the applied refactors. I suggest we continue to improve and modernise Argument Clinic, but now through more targeted issues; this issue has served its purpose. Thanks to everyone involved.

AA-Turner pushed a commit to AA-Turner/devguide that referenced this issue Sep 13, 2023
erlend-aasland added a commit to python/devguide that referenced this issue Sep 26, 2023
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

No branches or pull requests

2 participants