Skip to content

Conversation

@guicho271828
Copy link
Contributor

Mypy fails to check format variable passed around in backends due to the built-in global binding to format.
This PR renames it for private internal methods. No changes to the public API.
Merge this after #135 .
@jakelorocco

@mergify
Copy link

mergify bot commented Oct 1, 2025

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert|release)(?:\(.+\))?:

@guicho271828 guicho271828 force-pushed the masa/rename-format-argument-for-internal-methods branch 2 times, most recently from 536c1a7 to 8af76a9 Compare October 7, 2025 16:13
@nrfulton nrfulton self-requested a review October 10, 2025 16:53
Copy link
Contributor

@nrfulton nrfulton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we decide to go ahead with this, then lgtm. I wonder if there's a better way?

Copy link
Contributor

@jakelorocco jakelorocco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we even need to shadow the global format variable. Now that the functions have _format for their parameters, my IDE colors format differently than _format.

@guicho271828 guicho271828 force-pushed the masa/rename-format-argument-for-internal-methods branch from 365d3bc to db1e514 Compare October 12, 2025 01:07
@jakelorocco jakelorocco merged commit 7a6f780 into generative-computing:main Oct 16, 2025
4 checks passed
tuliocoppola pushed a commit to tuliocoppola/mellea that referenced this pull request Nov 5, 2025
generative-computing#172)

* refactor: renamed 'format' variable to '_format' in internal methods so that mypy detects it

* fix: use format = None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants