Skip to content

IMPROVEMENT: remove wildcard imports #258

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

Closed
melange396 opened this issue Sep 2, 2020 · 1 comment · Fixed by #349
Closed

IMPROVEMENT: remove wildcard imports #258

melange396 opened this issue Sep 2, 2020 · 1 comment · Fixed by #349
Labels
refactor Long-term projects to revise existing machinery

Comments

@melange396
Copy link
Contributor

using the wildcard import can introduce problems when debugging. it puts objects into scope without being declared or assigned, and hunting those down can be difficult. this can be even worse if multiple imports have name collisions.

for example, instead of: delphi_emr_hosp.constants import *
we can use: delphi_emr_hosp.constants import NA, SIGNALS
or even: from delphi_emr_hosp import constants as c # then use c.NA and c.SIGNALS

the choice is up to the implementer for brevity's or readability's sake.
there are many of these in the repo

@krivard krivard added the refactor Long-term projects to revise existing machinery label Sep 18, 2020
@chinandrew
Copy link
Contributor

emr_hosp is no longer active per footnote in #306, and the only other occurrences of these in non-test code are in the combo signal which I've hopefully addressed in #346. I see 3 other occurences: 2 in claims_hosp tests and 1 in cdc_covidnet tests, which should also be a quick fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Long-term projects to revise existing machinery
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants