Skip to content

Comments

[Analyzer Comments]: Changes for Upgrade to Python 3.13#2392

Merged
BethanyG merged 7 commits intoexercism:mainfrom
BethanyG:upgrade-to-python-3.13
Feb 9, 2026
Merged

[Analyzer Comments]: Changes for Upgrade to Python 3.13#2392
BethanyG merged 7 commits intoexercism:mainfrom
BethanyG:upgrade-to-python-3.13

Conversation

@BethanyG
Copy link
Member

@BethanyG BethanyG commented Feb 8, 2026

  • Added general recommendations for exercises that have no comments from analyzer.
  • Changed analyzer-comments/python/pylint/convention.md to (hopefully) accommodate additional code examples and comments from PyLint.
  • Changed analyzer-comments/python/pylint/refactor.md to (hopefully) accommodate additional code examples and comments from PyLint.

…Added general recommendations for exercises that have no comments from analyzer.
@github-actions github-actions bot added track/python Python track type/analyzer-comments Analyzer comments labels Feb 8, 2026
@BethanyG BethanyG requested review from IsaacG and kotp February 8, 2026 18:59
@BethanyG
Copy link
Member Author

BethanyG commented Feb 8, 2026

@kotp - need feedback on the Ruby string interpolation for the PyLint comments. I did try them out on https://try.ruby-lang.org/, but not completely sure that they'll work with Exercism's processing. I want to avoid having to PR all of the code examples here, which is why I am shoving them into JSON and then trying to expand them in the feedback doc. Please let me know if you need to see a sample analysis.json.

@IsaacG - I think we can safely do 10 recommendations in general_recommendations.md, but I'd love feedback on the last 4-6 as "general tips". Students would only see this feedback if they

  1. Had code that was good but not perfect.
  2. Didn't have any other analyzer feedback - either from PyLint or from the Analyzer.

Many thanks to you both for reviewing!!

@BethanyG
Copy link
Member Author

BethanyG commented Feb 8, 2026

Decided I should just put in a sample analysis.json. See below:

{
    "summary": "There are a few changes we'd like you to make before completing this exercise.",
    "comments": [
        {
            "comment": "python.pylint.warning",
            "params": {
                "lineno": "46",
                "code": "W0622 redefined-builtin",
                "message": "Redefining built-in 'sum'",
                "bad_code": "def map():  # [redefined-builtin]\n    pass\n",
                "good_code": "def map_iterable():\n    pass\n",
                "related_info": null,
                "details": "Shadowing [built-ins](https://docs.python.org/3.13/library/functions.html) at the global scope is discouraged because it\nobscures their behavior throughout the entire module, increasing the\nrisk of subtle bugs when the built-in is needed elsewhere.\n\n In contrast, local redefinitions _might_ be acceptable as their impact is confined to a\nspecific scope; although it is generally not a good idea.\n"
            },
            "type": "essential"
        },
        {
            "comment": "python.pylint.error",
            "params": {
                "lineno": "46",
                "code": "E0601 used-before-assignment",
                "message": "Using variable 'sum' before assignment",
                "bad_code": "print(hello)  # [used-before-assignment]\nhello = \"Hello World !\"\n",
                "good_code": "hello = \"Hello World !\"\nprint(hello)\n",
                "related_info": null,
                "details": null
            },
            "type": "essential"
        },
        {
            "comment": "python.pylint.refactor",
            "params": {
                "lineno": "62",
                "code": "R1705 no-else-return",
                "message": "Unnecessary \"elif\" after \"return\", remove the leading \"el\" from \"elif",
                "bad_code": "def compare_numbers(a: int, b: int) -> int:\n    if a == b:  # [no-else-return]\n        return 0\n    elif a < b:\n        return -1\n    else:\n        return 1\n",
                "good_code": "def compare_numbers(a: int, b: int) -> int:\n    if a == b:\n        return 0\n    if a < b:\n        return -1\n    return 1\n",
                "related_info": "- [Unnecessary-else-statements](https://www.pythonmorsels.com/unnecessary-else-statements/)\n",
                "details": null
            },
            "type": "actionable"
        }
    ]
}

@kotp
Copy link
Member

kotp commented Feb 9, 2026

It appears as though it should work.

But I have not worked with the analyzer much at all (can't even remember
the last time I played with it), so I can not say for certain.

That said, there is nothing that is raising the hairs on the back of my
neck. (Or my neck is going bald, which is also a possibility.)

kotp
kotp previously approved these changes Feb 9, 2026
IsaacG
IsaacG previously approved these changes Feb 9, 2026
Co-authored-by: Isaac Good <IsaacG@users.noreply.github.com>
@BethanyG BethanyG dismissed stale reviews from IsaacG and kotp via 64cd525 February 9, 2026 08:39
BethanyG and others added 2 commits February 9, 2026 00:40
Co-authored-by: Isaac Good <IsaacG@users.noreply.github.com>
Co-authored-by: Isaac Good <IsaacG@users.noreply.github.com>
@BethanyG BethanyG requested a review from IsaacG February 9, 2026 08:42
@BethanyG BethanyG requested a review from IsaacG February 9, 2026 20:46
@BethanyG
Copy link
Member Author

BethanyG commented Feb 9, 2026

Hopefully, these are the last touches. 😄

@BethanyG BethanyG merged commit bf58fef into exercism:main Feb 9, 2026
1 check passed
@BethanyG
Copy link
Member Author

BethanyG commented Feb 10, 2026

It appears as though it should work.

But I have not worked with the analyzer much at all (can't even remember the last time I played with it), so I can not say for certain.

That said, there is nothing that is raising the hairs on the back of my neck. (Or my neck is going bald, which is also a possibility.)

Looks as though it failed miserably.

Here is what made it to the UI:

image

Is that because the Analyzer is not yet passing the info? Any thoughts on what I could do to make the substitutions work? Maybe try % and not %Q?

Guess I will save any thrashing or updates until after the new Analyzer is merged - just in case it's the JSON.

@kotp
Copy link
Member

kotp commented Feb 10, 2026

I wonder if the mechanics of it knows how to use ERB for this.

You want to get together later today or tomorrow to look at this together?

But yes, for processing escapes you would use %Q

@BethanyG
Copy link
Member Author

I wonder if the mechanics of it knows how to use ERB for this.

You want to get together later today or tomorrow to look at this together?

But yes, for processing escapes you would use %Q

Sure - if we can simulate what the website is doing. I can also change what is returned in the JSON so that there aren't escapes. I just need to know how to format things to create a proper code fence in the output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

track/python Python track type/analyzer-comments Analyzer comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants