Skip to content

FIX: Avoid shadowing outer variable - #130

Merged
arokem merged 1 commit into
tee-ar-ex:masterfrom
jhlegarreta:fix/avoid-variable-shadowing
Aug 11, 2026
Merged

FIX: Avoid shadowing outer variable#130
arokem merged 1 commit into
tee-ar-ex:masterfrom
jhlegarreta:fix/avoid-variable-shadowing

Conversation

@jhlegarreta

Copy link
Copy Markdown
Contributor

Avoid shadowing outer variable. Fixes:

Shadows name 'data' from outer scope

Avoid shadowing outer variable. Fixes:
```
Shadows name 'data' from outer scope
```
@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.57%. Comparing base (51efc18) to head (7e8ad8d).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #130   +/-   ##
=======================================
  Coverage   63.57%   63.57%           
=======================================
  Files          12       12           
  Lines        2570     2570           
=======================================
  Hits         1634     1634           
  Misses        936      936           
Flag Coverage Δ
unittests 63.57% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@arokem

arokem commented Aug 11, 2026

Copy link
Copy Markdown
Member

Sorry, where is that being raised? Is that a warning? Should we put in a more rigorous check that it doesn't come back?

@jhlegarreta

Copy link
Copy Markdown
Contributor Author

Sorry, where is that being raised?

IDE. Pycharm in this case.

Is that a warning?

Yes. Most of the times Python deals with these correctly, and does not result in bugs. But I guess depending on the circumstances, this could result in unexpected behavior.

Should we put in a more rigorous check that it doesn't come back?

I do not know how to add a check for this out of the top of my head. Whatever IDE you use, it will probably find a long list of warnings that linters are not finding.

@arokem

arokem commented Aug 11, 2026

Copy link
Copy Markdown
Member

I don't mind this change, but I think your IDE might be catching something that isn't in the outer scope for this variable, but simply in another function. Nevertheless, this is innocuous, so merging.

@arokem
arokem merged commit 13b5d77 into tee-ar-ex:master Aug 11, 2026
16 checks passed
@jhlegarreta

Copy link
Copy Markdown
Contributor Author

I don't mind this change, but I think your IDE might be catching something that isn't in the outer scope for this variable, but simply in another function. Nevertheless, this is innocuous, so merging.

The IDE complains because the function where the private function resides is already using a data variable. I think it is a legit complaint.

@jhlegarreta
jhlegarreta deleted the fix/avoid-variable-shadowing branch August 11, 2026 17:42
@arokem

arokem commented Aug 11, 2026

Copy link
Copy Markdown
Member

Ah yes, I see it now. Objection retracted.

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.

2 participants