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

Departure Time Window Default and Warning #304

Merged
merged 6 commits into from
Aug 10, 2023

Conversation

wklumpen
Copy link
Contributor

@wklumpen wklumpen commented Aug 2, 2023

See #292

@christophfink
Copy link
Member

Something had gone wrong with black - maybe the IDE you used ran a different linter? I quickly ran black and pushed a re-linted version so the pipeline can run and finish before it’s office hours again for you :)

@christophfink
Copy link
Member

The test data sets don’t match any more (of course, arguments changed). Two options: 1) change the tests to set departure_time_window = datetime.timedelta(hours=1) to mimick the old default behaviour, or 2) regenerate the test comparison data sets

@wklumpen
Copy link
Contributor Author

wklumpen commented Aug 3, 2023

I would vote making the tests fixed time windows

@christophfink
Copy link
Member

I would vote making the tests fixed time windows

I’m fine with either way, choose what is easier for you :)

@wklumpen wklumpen linked an issue Aug 8, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #304 (5a07e50) into main (a2ca3e7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #304   +/-   ##
=======================================
  Coverage   99.17%   99.17%           
=======================================
  Files          34       34           
  Lines        1326     1328    +2     
=======================================
+ Hits         1315     1317    +2     
  Misses         11       11           
Files Changed Coverage Δ
src/r5py/r5/regional_task.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@christophfink christophfink left a comment

Choose a reason for hiding this comment

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

Looks good :)

@wklumpen wklumpen merged commit a447e34 into r5py:main Aug 10, 2023
28 checks passed
@wklumpen wklumpen deleted the 292-departure-time-window branch August 10, 2023 15:22
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.

Default value for departure_time_window
2 participants