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

Scada interface #318

Merged
merged 12 commits into from Oct 23, 2020
Merged

Conversation

WenzDaniel
Copy link
Collaborator

@WenzDaniel WenzDaniel commented Sep 11, 2020

What is the problem / what does the code in this PR do
In this PR we make some changes to the time range querying via strax.

Can you briefly describe how it works?
I slightly modified estimate_run_start such that it returns the run endtime as well.
Further to_absolute_time_range was modified such that it returns the run endtime in case no time range is selected.

Can you give a minimal working example (or illustrate with a figure)?
At the end of this notebook briefly show the changes.

@WenzDaniel WenzDaniel marked this pull request as ready for review October 23, 2020 07:06
Reset mongo registry for chunk_i == 0 and format comments
Copy link
Member

@JoranAngevaare JoranAngevaare left a comment

Choose a reason for hiding this comment

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

Seems clean and straigtforward!

sorry for 49c0674 it was just a super minor change and some reformating

(seconds_range is None) +
(time_within is None) +
(full_range is None))
if selection < 2:
Copy link
Member

Choose a reason for hiding this comment

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

@WenzDaniel , could you make full_range a bool and update the Runtime error?
As in :
full_range=False # as kwarg
full_range==False # in selection comparison

and
" time_range, seconds_range, time_within or full_range" # in the Runtime error

Copy link
Member

Choose a reason for hiding this comment

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

I think a bool is cleaner for an option but it's not too relevant

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I corrected the raise message. About the other thing, first I thought so too. But I think Jelle did this intentionally to allow arrays as input. Otherwise things like

if np.array([start, stop]) :

would give an ambiguity error and one would have to change it in something like:

if np.any(selection) :

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.

None yet

2 participants