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.

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

Allow null event range #854

Merged
merged 6 commits into from
Oct 10, 2024
Merged

Allow null event range #854

merged 6 commits into from
Oct 10, 2024

Conversation

gonzaponte
Copy link
Collaborator

This PR adds the option to run a city with event_range = 0, which can be useful for testing.

Closes #573

@gonzaponte gonzaponte force-pushed the null-event-range branch 4 times, most recently from db81df1 to bd7de53 Compare May 15, 2024 13:11
@gonzaponte gonzaponte requested a review from jahernando May 15, 2024 14:19
@gonzaponte gonzaponte requested review from jahernando and removed request for jahernando October 2, 2024 14:20
@gonzaponte
Test that all cities accept event_range = 0
4c9c9bf 
This demonstrates the issue on the bigger picture
@gonzaponte
State close_all explicitly in test_slice_downstream
cb4332e 
Just to make it more clear when compared with the test that is going
to be included next
@gonzaponte
Add test that verifies null slice case with close_all
bf664a8 
This demonstrates the bug that induces the problem demonstrated before
@gonzaponte
Add protection against null slice case with close_all
77453ff
@gonzaponte
Rerganize variables
4e9a41e 
note that sorting is somehow important for pytest
@gonzaponte
Speed-up test by running over 0 events
b5442c3 
For this test, we don't need to run over any events,as the output
structure is fixed beforehand
martinperezmaneiro
martinperezmaneiro approved these changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@martinperezmaneiro martinperezmaneiro left a comment

Choose a reason for hiding this comment

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

Good PR. Adds protection to slice function for event_range = 0, and creates the corresponding test. Also changes a file structure test to run over no events. Everything works as expected :)

@carhc carhc merged commit fefdb57 into next-exp:master Oct 10, 2024
1 check passed
@gonzaponte gonzaponte deleted the null-event-range branch October 10, 2024 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinperezmaneiro martinperezmaneiro martinperezmaneiro approved these changes

@jahernando jahernando Awaiting requested review from jahernando

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

event_range = 0 doesn't work
3 participants
@gonzaponte @martinperezmaneiro @carhc