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

Corrected typo without breaking backwards compatibility #3398

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

nuvious
Copy link

@nuvious nuvious commented Feb 20, 2025

I noticed the note about the misspelled variable in #3029 which was mentioned in #2361 and #1861. The concerns about breaking backwards compatibility were important and actually sparked interest if this was resolvable. I sought to solve this issue while meeting the following requirements:

  1. Does not introduce a breaking change
  2. Informs the user via a warning of the deprecation ONLY if used in a named import
  3. Does not make promises about when the old name will be removed; just mentions future release
  4. Does not warn the user on general module imports

I used a mix of inspect functionality and very basic regex to achieve the above to the best of my understanding pending your review.

Python 3.8-3.12 local test using tox below; had a residual 3.13 environment that made me have to run it separately:
image

Python 3.13 test run separately after cleaning the 3.13 environment:
image

Ran pre-commit per the Contribution guide but it seemed to skip all the tests for some reason:

image

Also ran ruff independently on edited files:
image

Ran isort and neither file edited was modified, so that's good as well.
image

Below is the final result in terms of user experience if they attempt a module or named import:

image

May be a bit trivial, but below shows imports of the old variable name have changes reflected in the corrected one via pass by reference convention:

image

Standing by for any questions, comments, recommendations or direction.

…ack unnecessarily. Also reversed the search since imports tend to be low on the stack.
…up in the inspect stack; ended up being erroneous.
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.

1 participant