Skip to content

STY: Avoid using package name for variable or parameter names - #148

Merged
arokem merged 1 commit into
tee-ar-ex:mainfrom
jhlegarreta:sty/avoid-shadowing-library-name
Sep 22, 2026
Merged

arokem merged 1 commit into
tee-ar-ex:mainfrom
jhlegarreta:sty/avoid-shadowing-library-name

Conversation

@jhlegarreta

Copy link
Copy Markdown
Contributor

Avoid using package name for variable or parameter names: do not use trx as the name for parameters or variables.

@jhlegarreta

jhlegarreta commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Although I liked the compactness of trx as a variable name, I do not believe that using the library name to name variables is good practice. I am not particularly happy with the ttgrm name, so I am open to suggestions. I have not modified other names that use trx as a substring, like trx1, trx_copy, etc. but can do if we agree on a better naming.

@jhlegarreta
jhlegarreta force-pushed the sty/avoid-shadowing-library-name branch 4 times, most recently from 983d1f8 to 2c120d0 Compare September 20, 2026 16:46
@codecov

codecov Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.77990% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.40%. Comparing base (23d13b2) to head (494fff0).

Files with missing lines Patch % Lines
trx/trx_file_memmap.py 90.32% 9 Missing ⚠️
trx/cli.py 60.00% 2 Missing ⚠️
trx/workflows.py 93.33% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #148   +/-   ##
=======================================
  Coverage   86.40%   86.40%           
=======================================
  Files          15       15           
  Lines        3031     3031           
=======================================
  Hits         2619     2619           
  Misses        412      412           
Flag Coverage Δ
unittests 86.40% <93.77%> (ø)

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.

@jhlegarreta
jhlegarreta force-pushed the sty/avoid-shadowing-library-name branch 3 times, most recently from 1b5d011 to 5f11494 Compare September 20, 2026 16:53
@arokem

arokem commented Sep 20, 2026

Copy link
Copy Markdown
Member

What about 'tgm'?

@arokem

arokem commented Sep 20, 2026

Copy link
Copy Markdown
Member

Even 'tg'

@skoudoro

Copy link
Copy Markdown
Collaborator

Why an acronym when you can just use the full name to be explicit ?

@jhlegarreta

Copy link
Copy Markdown
Contributor Author

Why an acronym when you can just use the full name to be explicit ?

I preferred to shorten it as I considered tractogram overly long for a variable name.

@jhlegarreta

Copy link
Copy Markdown
Contributor Author

Between tg and tgm I think I prefer the latter.

@jhlegarreta
jhlegarreta force-pushed the sty/avoid-shadowing-library-name branch from 5f11494 to 704411a Compare September 22, 2026 16:45
Avoid using package name for variable or parameter names: do not use
`trx` as the name for parameters or variables.
@jhlegarreta
jhlegarreta force-pushed the sty/avoid-shadowing-library-name branch from 704411a to 494fff0 Compare September 22, 2026 16:48
@jhlegarreta

Copy link
Copy Markdown
Contributor Author

Adopted tgm. This is ready to go on my end.

@arokem

arokem commented Sep 22, 2026

Copy link
Copy Markdown
Member

Thanks!

@arokem
arokem merged commit 42fb495 into tee-ar-ex:main Sep 22, 2026
20 checks passed
@jhlegarreta
jhlegarreta deleted the sty/avoid-shadowing-library-name branch September 22, 2026 17:01
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