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

[MRG] LGTM warnings: Variable defined multiple times #1493

Conversation

DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Aug 30, 2021

Describe the changes

Fix LGTM warnings:
https://lgtm.com/projects/g/pydicom/pydicom/snapshot/283a1a1994d3d01b4d1e1fb3959c4de35c3240a8/files/source/generate_cids/generate_concept_dicts.py#x9b38b497e036fd51:1
https://lgtm.com/projects/g/pydicom/pydicom/snapshot/283a1a1994d3d01b4d1e1fb3959c4de35c3240a8/files/source/generate_cids/generate_concept_dicts.py#xe1a70f817f3b0bb8:1

I have kept the last definitions of the two functions - the definitions currently used.

Tasks

  • Unit tests added that reproduce the issue or prove feature is working
  • Fix or feature added
  • Code typed and mypy shows no errors
  • Documentation updated (if relevant)
    • No warnings during build
    • Preview link (CircleCI -> Artifacts -> doc/_build/html/index.html)
  • Unit tests passing and overall coverage the same or better

@DimitriPapadopoulos DimitriPapadopoulos changed the title LGTM warnings: Variable defined multiple times [MRG] LGTM warnings: Variable defined multiple times Aug 30, 2021
@codecov
Copy link

codecov bot commented Aug 30, 2021

Codecov Report

Merging #1493 (44f97c5) into master (19fdba2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1493   +/-   ##
=======================================
  Coverage   97.42%   97.42%           
=======================================
  Files          66       66           
  Lines       10183    10183           
=======================================
  Hits         9921     9921           
  Misses        262      262           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19fdba2...44f97c5. Read the comment docs.

@@ -210,67 +210,6 @@ def get_table_d1():


def write_concepts(concepts, cid_concepts, cid_lists, name_for_cid):
lines = (DOC_LINES +
Copy link
Member

Choose a reason for hiding this comment

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

Better use this version, as it uses DOC_LINES for the duplicated part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's right. Fixed now.

This assignment to 'write_concepts' is unnecessary as it is redefined here before this value is used.
This assignment to 'write_snomed_mapping' is unnecessary as it is redefined here before this value is used.

https://lgtm.com/rules/1800095/
@mrbean-bremen mrbean-bremen merged commit 860497e into pydicom:master Aug 31, 2021
@DimitriPapadopoulos DimitriPapadopoulos deleted the variable_defined_multiple_times branch August 31, 2021 05:53
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.

None yet

2 participants