Skip to content

Bug fix for lat lon #50

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Aug 24, 2023
Merged

Bug fix for lat lon #50

merged 4 commits into from
Aug 24, 2023

Conversation

egrace479
Copy link
Member

  • Checks that latitude is within [-90,90] and longitude is within [-180,180], otherwise labels the outliers as unknown.
  • Returns error if either column contains a data type that can't be converted to a float.
  • Explanation of this has been added to README (with type and range explanation).
  • Closes Issue Single errors in lat and lon break the map #48

@egrace479 egrace479 added the bug Something isn't working label Aug 23, 2023
johnbradley
johnbradley reviewed Aug 23, 2023
View reviewed changes
@johnbradley
Copy link
Contributor

@egrace479 Since this is a bug fix could you add a unit test? It looks like you added two sample csv files that you could be used to test the logic.

johnbradley
johnbradley approved these changes Aug 24, 2023
View reviewed changes
Copy link
Contributor

@johnbradley johnbradley left a comment

Choose a reason for hiding this comment

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

LGTM

thompsonmj
thompsonmj requested changes Aug 24, 2023
View reviewed changes
@thompsonmj thompsonmj self-requested a review August 24, 2023 17:53
thompsonmj
thompsonmj approved these changes Aug 24, 2023
View reviewed changes
Copy link
Contributor

@thompsonmj thompsonmj left a comment

Choose a reason for hiding this comment

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

👍

@egrace479 egrace479 merged commit c14bd04 into dev Aug 24, 2023
egrace479 added a commit to Imageomics/telemetry-dashboard that referenced this pull request Aug 24, 2023
@egrace479
Updated filtering of lat lon to use Series.between and added lat-lon …
5ba7271 
…bug fix from dashboard-prototype (Imageomics#50).
@egrace479 egrace479 mentioned this pull request Sep 6, 2023
Closed
@egrace479 egrace479 deleted the bug-fix/lat-lon branch September 6, 2023 16:28
@egrace479 egrace479 mentioned this pull request Sep 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johnbradley johnbradley johnbradley approved these changes

@thompsonmj thompsonmj thompsonmj approved these changes

@hlapp hlapp Awaiting requested review from hlapp

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@egrace479 @johnbradley @thompsonmj