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

Generation/pass frequency #157

Merged
merged 6 commits into from
Jul 27, 2020
Merged

Generation/pass frequency #157

merged 6 commits into from
Jul 27, 2020

Conversation

Kostiiii
Copy link
Contributor

Added freq passing to couple of timeseries generating function to allow creating smaller timeseries + update tests to check if it possible to create timeseries with those lengths.

@Kostiiii Kostiiii requested review from hrzn and TheMP as code owners July 21, 2020 10:46
@Kostiiii Kostiiii requested a review from pennfranc July 21, 2020 10:47
self.assertAlmostEqual(linear_ts.values()[-1][0] - linear_ts.values()[-2][0],
(end_value - start_value) / (length - 1))

for length in range(2, 50):
Copy link
Contributor

Choose a reason for hiding this comment

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

seems to be doing much more iterations now, how much slower is it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also - do we need to go over all interegs from 2 to 50 or is it enough to pick up start, middle and end point?

Copy link
Contributor Author

@Kostiiii Kostiiii Jul 21, 2020

Choose a reason for hiding this comment

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

I feel like we don't have worry about it too much since this tests are really fast. But it is probably enough to test few options here so will change it to your proposition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the record github actions were around 30 seconds slower so in the end it was worth it to change it.

Copy link
Contributor

@hrzn hrzn left a comment

Choose a reason for hiding this comment

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

thanks!

@TheMP TheMP merged commit 64f34c2 into develop Jul 27, 2020
@Kostiiii Kostiiii deleted the generation/pass_frequency branch July 27, 2020 08:09
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.

3 participants