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

Duration Import Bugfix #1212

Merged
merged 1 commit into from
Jul 8, 2023
Merged

Conversation

chrisrosset
Copy link
Collaborator

This change makes the import look at duration and distance separately and imports the value provided by the user even if only one of them is provided.

Fixes #162.

This change makes the import look at duration and distance separately
and imports the value provided by the user even if only one of them
is provided.

Fixes jpatokal#162.
@chrisrosset
Copy link
Collaborator Author

Fixes #523 too.

@reedy
Copy link
Collaborator

reedy commented Jun 29, 2023

Any chance of before and after screenshots? :)

@chrisrosset
Copy link
Collaborator Author

Any chance of before and after screenshots? :)

Happy to oblige.

Test Case Overview

We have 4 cases:

  1. Distance and duration both present
  2. Distance but no duration
  3. Duration but no distance
  4. Both distance and duration missing

Test CSV

I have created a CSV with four JFK-LHR flights which map to our test cases. On its own, OpenFlights calculates the distance as 3440 miles and the duration as 07:22. I have purposefully chosen slightly different values for the user-provided values to make them easier to distinguish.

Date,From,To,Flight_Number,Airline,Distance,Duration,Seat,Seat_Type,Class,Reason,Plane,Registration,Trip,Note,From_OID,To_OID,Airline_OID,Plane_OID
2023-01-01,JFK,LHR,,Unknown,3500,07:45,,,Y,L,,,,,,,,
2023-01-01,JFK,LHR,,Unknown,3450,,,,Y,L,,,,,,,,
2023-01-01,JFK,LHR,,Unknown,,07:42,,,Y,L,,,,,,,,
2023-01-01,JFK,LHR,,Unknown,,,,,Y,L,,,,,,,,

Download link: pr-1212-duration-distance.csv

Before

pr-1212-before

Test cases 1 and 4 behave correctly:

  • 1 shows user provided values from the CSV
  • 4 shows synthetic values

Test cases 2 and 3 show undesirable behavior. The user provided values have been discarded and synthetic values are shown for both distance and duration.

After

pr-1212-after

  • Test cases 1 and 4 behave as previously.
  • Test case 2 now uses the distance from the CSV
  • Test case 3 now uses the duration from the CSV

@reedy reedy merged commit 4f5f072 into jpatokal:master Jul 8, 2023
1 check passed
@chrisrosset chrisrosset deleted the gh-162-duration-import branch July 8, 2023 17: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.

[Bug] Duration not taken in account when importing CSV [sf#162]
2 participants