Skip to content

EMR: standard signal names #250

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

Merged
merged 11 commits into from
Sep 3, 2020
Merged

EMR: standard signal names #250

merged 11 commits into from
Sep 3, 2020

Conversation

vishakha1812
Copy link
Contributor

No description provided.

Copy link
Contributor

@melange396 melange396 left a comment

Choose a reason for hiding this comment

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

a few style things and some nits (i apologize, i have OCD).

if you intend for the two copies of 02_20_uszips.csv to be identical, perhaps they should symlink to each other to keep stuff in sync?

otherwise, it looks good and the logic is sound!

@krivard
Copy link
Contributor

krivard commented Sep 2, 2020

reminder to let us know whether unit tests pass --

Copy link
Contributor

@melange396 melange396 left a comment

Choose a reason for hiding this comment

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

LGTM! (assuming all tests pass, of course)

@dshemetov
Copy link
Contributor

dshemetov commented Sep 2, 2020

Tests all pass on my end. Sorry I can't look at the code deeply this time!

@krivard krivard merged commit 008bd9b into main Sep 3, 2020
@krivard krivard deleted the wip_emr branch October 29, 2020 18:11
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.

4 participants