Add protections for the get_data utility #293

alurban · 2019-04-03T16:44:51Z

This PR fixes a bug in the get_data utility by adding protections against nonexistent frame files when attempting to build a frame file cache, and falling back to TimeSeries{Dict}.get rather than fetch when that fails. Unit tests are also updated to reflect these changes.

Other changes include:

Remove redundant obs keyword argument from get_data, and infer the observatory from the frametype (when applicable) using a regular expression
Remove references to obs from a few scripts
Simplify a couple of unit tests
Update docstrings
Misc. typo fix in gwdetchar-conlog

cc @duncanmmacleod

duncanmmacleod

I'm confused as to why all of the tools now call out to gwdatafind. It looks like the returned file lists aren't being reused (which would be a good reason to call find_urls upfront), so can they not just let gwpy do the gwdatafind call for them?

duncanmmacleod · 2019-04-03T16:51:23Z

gwdetchar/io/datafind.py

    # read from frames or NDS
-    if source:
+    if source is not None:
        if isinstance(channel, (list, tuple)):
            channel = remove_missing_channels(channel, source)
        return series_class.read(
            source, channel, start=start, end=end, nproc=nproc,
            verbose=verbose, **kwargs)
    elif isinstance(channel, str):


it occurs to me that this should probably be

elif not isinstance(channel, (list, tuple))

to match the condition above that declares series_class

thanks @duncanmmacleod, I've changed this in the latest commit. I also restored the logic that attempts to load a cache using gwdatafind because I remembered that it's much simpler that way, but added some protection against non-existent frame files.

coveralls · 2019-04-03T18:46:58Z

Coverage decreased (-0.1%) to 95.398% when pulling f047644 on alurban:patch-get-data into 5ef647c on gwdetchar:master.

coveralls · 2019-04-03T18:46:58Z

Coverage decreased (-0.1%) to 95.398% when pulling f047644 on alurban:patch-get-data into 5ef647c on gwdetchar:master.

coveralls · 2019-04-03T18:46:59Z

Coverage decreased (-0.1%) to 95.398% when pulling f047644 on alurban:patch-get-data into 5ef647c on gwdetchar:master.

coveralls · 2019-04-03T18:46:59Z

Coverage decreased (-0.1%) to 95.398% when pulling f047644 on alurban:patch-get-data into 5ef647c on gwdetchar:master.

coveralls · 2019-04-03T18:47:00Z

Coverage decreased (-0.04%) to 95.472% when pulling a1e4921 on alurban:patch-get-data into a801697 on gwdetchar:master.

…the unit tests for get_data

alurban · 2019-04-03T19:48:21Z

@duncanmmacleod, I just saw your longer comment, sorry about that. Response below:

I prefer to have gwdetchar.io.datafind.get_data attempt to create its own file cache before falling back to TimeSeries.get, mainly because it then becomes possible to sieve the file cache against requested channels that actually exist in frame files. This makes gwdetchar tools extremely robust to changes in channel names, which is very common between observing runs.

For example, gwdetchar-scattering is written to analyze optical squeezer suspensions by default, but it will still run successfully (with default options) over O1 or O2 data when those squeezer optics didn't exist. Likewise for gwdetchar-omega, whose default channel list configuration changed several times just in the first few months of 2019.

duncanmmacleod · 2019-04-04T14:39:50Z

gwdetchar/io/datafind.py

        if isinstance(channel, (list, tuple)):
            channel = remove_missing_channels(channel, source)
        return series_class.read(
            source, channel, start=start, end=end, nproc=nproc,
            verbose=verbose, **kwargs)
-    elif isinstance(channel, str):
-        return series_class.fetch(
+    except (HTTPError, TypeError):  # frame files not found


Can you explain when an HTTPError would be raised?

@duncanmmacleod, the HTTPError is raised whenever gwdatafind.find_urls can't locate frame files (you get a 400 Bad Request).

I think that if you don't pass obs and the first letter of frametype is not a meaningful observatory code (e.g. for SenseMon frames), this will also lead to HTTPError. The TypeError case comes when you pass neither frametype nor source. In all these cases, the idea is that it would then fall back to TimeSeries.get with no protection against missing channels in frame files.

@duncanmmacleod, I stand corrected, if you attempt to read SenseMon frames with frametype='SenseMonitor_hoft_L1_M' and obs=None then the result is an IORegistryError: Format could not be identified. Passing frametype=None falls back to TimeSeries.get, and passing both frametype and obs successfully uses TimeSeries.read.

alurban · 2019-04-05T02:43:11Z

@duncanmmacleod, I've fixed this so that the obs kwarg is no longer needed, the observatory is now inferred from frametype (when it's passed) using a regular expression. Also, whenever neither frametype nor source are passed, the utility will fall back to TimeSeries.get.

I've also updated the docstring to [hopefully] make it clearer to the user which arguments do what, and when.

duncanmmacleod

Aye, looks good.

alurban added enhancement bug-fix omega scans lasso scattering labels Apr 3, 2019

alurban added this to the 0.4.1 milestone Apr 3, 2019

alurban self-assigned this Apr 3, 2019

alurban requested a review from duncanmmacleod April 3, 2019 16:44

duncanmmacleod requested changes Apr 3, 2019

View reviewed changes

alurban changed the title ~~Simplify the get_data utility~~ WIP: Add protections for the get_data utility Apr 3, 2019

alurban force-pushed the patch-get-data branch 2 times, most recently from 3ba8ff0 to 2407255 Compare April 3, 2019 17:55

alurban changed the title ~~WIP: Add protections for the get_data utility~~ Add protections for the get_data utility Apr 3, 2019

alurban force-pushed the patch-get-data branch 2 times, most recently from 57f7b0e to f047644 Compare April 3, 2019 18:38

alurban force-pushed the patch-get-data branch 2 times, most recently from 55ff36f to be4e43b Compare April 3, 2019 19:01

alurban requested a review from duncanmmacleod April 3, 2019 19:03

Add protection against local frame files not being found, and update …

89aeffb

…the unit tests for get_data

alurban force-pushed the patch-get-data branch from be4e43b to 89aeffb Compare April 3, 2019 19:16

Remove unneeded unit test

04a8dce

alurban removed lasso omega scans scattering labels Apr 4, 2019

duncanmmacleod reviewed Apr 4, 2019

View reviewed changes

alurban requested a review from duncanmmacleod April 5, 2019 02:43

Handle use cases more robustly

255b5ef

alurban force-pushed the patch-get-data branch from e7ad3d7 to 255b5ef Compare April 5, 2019 05:01

Alexander Urban added 3 commits April 5, 2019 08:45

Invert try-except block

d8fbd74

Gracefully handle a bad frametype

f2b9f1b

More careful unit tests for observatory frametype

a1e4921

alurban force-pushed the patch-get-data branch from ca4a2e0 to a1e4921 Compare April 5, 2019 21:25

duncanmmacleod approved these changes Apr 9, 2019

View reviewed changes

alurban merged commit 7e0070b into gwdetchar:master Apr 9, 2019

alurban deleted the patch-get-data branch April 9, 2019 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add protections for the get_data utility #293

Add protections for the get_data utility #293

alurban commented Apr 3, 2019 •

edited

duncanmmacleod left a comment

duncanmmacleod Apr 3, 2019

alurban Apr 3, 2019 •

edited

coveralls commented Apr 3, 2019

coveralls commented Apr 3, 2019

coveralls commented Apr 3, 2019

coveralls commented Apr 3, 2019

coveralls commented Apr 3, 2019 •

edited

alurban commented Apr 3, 2019

duncanmmacleod Apr 4, 2019

alurban Apr 4, 2019

alurban Apr 4, 2019 •

edited

alurban Apr 4, 2019

alurban commented Apr 5, 2019

duncanmmacleod left a comment

Add protections for the get_data utility #293

Add protections for the get_data utility #293

Conversation

alurban commented Apr 3, 2019 • edited

duncanmmacleod left a comment

Choose a reason for hiding this comment

duncanmmacleod Apr 3, 2019

Choose a reason for hiding this comment

alurban Apr 3, 2019 • edited

Choose a reason for hiding this comment

coveralls commented Apr 3, 2019

coveralls commented Apr 3, 2019

coveralls commented Apr 3, 2019

coveralls commented Apr 3, 2019

coveralls commented Apr 3, 2019 • edited

alurban commented Apr 3, 2019

duncanmmacleod Apr 4, 2019

Choose a reason for hiding this comment

alurban Apr 4, 2019

Choose a reason for hiding this comment

alurban Apr 4, 2019 • edited

Choose a reason for hiding this comment

alurban Apr 4, 2019

Choose a reason for hiding this comment

alurban commented Apr 5, 2019

duncanmmacleod left a comment

Choose a reason for hiding this comment

alurban commented Apr 3, 2019 •

edited

alurban Apr 3, 2019 •

edited

coveralls commented Apr 3, 2019 •

edited

alurban Apr 4, 2019 •

edited