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

fetch data sets based on run_id. Modif. in select_runs #192

Merged
merged 3 commits into from Jun 26, 2019

Conversation

mlotfiben
Copy link
Contributor

allow for selection of data set by giving one specific run_id and also it can be used with matching a given numbers of run_ids (example: run_id="110*").

@JelleAalbers
Copy link
Member

JelleAalbers commented Jun 20, 2019

Thanks Lotfi. It currently crashes because there is not yet a run_id argument in the select_runs function. You can add one with a default of None (and update the docstring).

It seems the logic for run_id is very similar to that of run_mode. Maybe it would work to replace both of these with one for loop, like so:

for field_name, pattern in (
        ('run_id', run_id), ('mode', run_mode)):

    values = dsets[field_name].values
    mask = np.zeros(len(values), dtype=np.bool_)
    for i, x in enumerate(values):
        if pattern_type == 'fnmatch':
            mask[i] = fnmatch.fnmatch(x, pattern)
        elif pattern_type == 're':
            mask[i] = bool(re.match(pattern, x))

    dsets = dsets[mask]

but I have not tested this.

@mlotfiben
Copy link
Contributor Author

mlotfiben commented Jun 20, 2019 via email

@JelleAalbers
Copy link
Member

JelleAalbers commented Jun 20, 2019

Ah sorry, in the loop we would have to add a check for None, otherwise indeed you'd have to give both.

for field_name, pattern in (
        ('run_id', run_id), ('mode', run_mode)):
    if pattern is None:
        continue

Perhaps there are other modifications too (I did not test the loop code), but it would be great to avoid code duplication.

Speaking of testing, could you edit the unit test here https://github.com/AxFoundation/strax/blob/master/tests/test_core.py#L252 to test that this new feature works?

Copy link
Member

@JelleAalbers JelleAalbers left a comment

Choose a reason for hiding this comment

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

Thanks Lotfi! We discovered the unit test for select_runs is actually broken (it passes tautologically), I will fix it after merging this.

@JelleAalbers JelleAalbers merged commit 2bf01be into AxFoundation:master Jun 26, 2019
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