General opener, using file info. #198

mhvk · 2018-04-04T01:12:29Z

My fun project for the evening. It does somewhat work, though clearly at least the sample rate should be checked:

In [1]: from baseband.data import SAMPLE_DADA, SAMPLE_VDIF, SAMPLE_MARK4, SAMPLE_MARK5B; import baseband

In [2]: baseband.file_info(SAMPLE_DADA)
Out[2]: 
{'fmt': 'dada',
 'sample_shape': SampleShape(npol=2, nchan=1),
 'start_time': <Time object: scale='utc' format='mjd' value=56475.06898148148>}

In [3]: baseband.open(SAMPLE_MARK4)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-e1748998f20e> in <module>()
----> 1 baseband.open(SAMPLE_MARK4)

/home/mhvk/packages/baseband/baseband/core.py in open(name, mode, fmt, **kwargs)
     38         if 'missing' in info and 's' in mode:
     39             raise ValueError("file format is {}, but required arguments {} "
---> 40                              "are missing.".format(fmt, info['missing']))
     41 
     42     module = importlib.import_module('.' + fmt, package='baseband')

ValueError: file format is mark4, but required arguments ['decade', 'ref_time'] are missing.

In [4]: baseband.open(SAMPLE_MARK4, decade=2010)
Out[4]: 
<Mark4StreamReader name=/home/mhvk/packages/baseband/baseband/data/sample.m4 offset=0
    sample_rate=32.0 MHz, samples_per_frame=80000,
    sample_shape=SampleShape(nchan=8), bps=2,
    start_time=2014-06-16T07:38:12.47500>

mhvk · 2018-04-04T18:55:41Z

I separated out file_info since it seemed obvious that would be the easier one to start with.

mhvk · 2018-04-16T00:31:56Z

Rebased this. The opener is now such a minor contribution that one might as well look at this one... (especially as I changed the file_info function...).

mhvk · 2018-04-20T18:53:01Z

I think this is getting ready for prime time!

mhvk · 2018-04-24T16:21:13Z

OK, with some further cleanup, I think this is ready. But probably good to have a thorough review, since this new baseband.open function might well become the standard way files are opened (which is partially why I now check consistency for any argument passed in).

cczhu

Highly incomplete review - will add more tonight.

cczhu · 2018-05-03T20:21:21Z

baseband/core.py

-    """
-    if format is None:
-        format = tuple(FILE_FORMATS.keys())
+    All keyword arguments passed in and are classified, ending up in one of


"passed in and are classified" -> "passed in are classified"

cczhu · 2018-05-03T20:42:00Z

baseband/core.py

+        elif key == 'nchan':
+            sample_shape = info_dict.get('sample_shape')
+            if sample_shape is not None:
+                # If we passed nchan, and info doesn't have it, but does have a


Not sure I'm happy with nchan being treated like the total size of the sample shape - couldn't the user, for a two channel, two thread file pass nthread = 2, nchan = 2? In that case, this check would return inconsistent.

I guess one could allow if if either sample_shape.nchan == nchan or sample_shape.nchan == 1 and the product equals nchan. I guess for a consistency check one does not have to be all that strict.

Actually, on second thought I think what's below is fine (I was pretty sleep-deprived when looking at this yesterday).

I still changed it to be a bit more generous: sample_shape.nchan == nchan should be OK too.

cczhu

Overall looks good, and I found no problems reading some sample files of various formats. The only non-nitpick I have is I'd like it to be recorded somewhere (maybe in Getting Started?) that only GSB timestamp files are recognized by the file info and general opener routines.

cczhu · 2018-05-04T17:28:38Z

baseband/core.py

+        elif key == 'nchan':
+            sample_shape = info_dict.get('sample_shape')
+            if sample_shape is not None:
+                # If we passed nchan, and info doesn't have it, but does have a


Actually, on second thought I think what's below is fine (I was pretty sleep-deprived when looking at this yesterday).

cczhu · 2018-05-04T18:16:08Z

baseband/tests/test_core.py

+
+
+@pytest.mark.parametrize('sample', (SAMPLE_M4, SAMPLE_M5B))
+def test_open_missing_args(sample):


Maybe:

def test_open_missing_args(sample): with pytest.raises(TypeError) as excinfo: baseband_open(sample, 'rs') assert "missing required arguments" in str(excinfo.value)

cczhu · 2018-05-04T18:20:51Z

baseband/tests/test_core.py

+    # reader, but then fail on an incorrect sample rate.
+    mark4_args = {'nchan': 8,
+                  'ref_time': Time('2014-01-01')}
+    with pytest.raises(ValueError):  # wrong sample_rate


As above, would be nice to also test that "arguments inconsistent" or "got unexpected keyword" show up in the error messages. I don't think it's essential to check these (or the above), but I feel it's more precise and also helps a bit when reading the tests at a later date.

Given that I felt the need to add comments to the tests, this is a good point. I made the checks somewhat minimal, though, as I don't like to be stuck with particular error messages.

cczhu · 2018-05-04T18:25:54Z

CHANGES.rst

+- Added a general file opener, ``baseband.open`` which for a set of formats
+  will check whether the file is of that format, and then load it using the
+  corresponding module. [#198]
+
 API Changes
 -----------



In "Other Changes and Additions" should mention that sample data files now listed in the documentation.

cczhu · 2018-05-04T18:26:11Z

docs/data/index.rst

+.. _data:
+
+*****************
+Sample data files


"Sample data files" -> "Sample Data Files"

we're not consistent with the capitalization (top level index.rst doesn't do it), but changed anyway.

cczhu · 2018-05-04T18:30:23Z

docs/index.rst

@@ -51,15 +51,17 @@ troubleshooting help and APIs for each.
 Core framework and utilities


Nothing to do with this PR, but seems silly not to fix considering the table of contents is being modified anyway: in the Overview section "authors_for_sphinx" is still listed, which repeats the authors and contributors listed under Project Details.

cczhu · 2018-05-04T18:31:54Z

docs/tutorials/getting_started.rst

-This tutorial covers the basic features of Baseband.  It assumes that
-`NumPy <http://www.numpy.org/>`_ and the `Astropy`_ units module have been
-imported::
+For some file formats, one can simply import baseband and use `baseband.open` to


This is great!

cczhu · 2018-05-04T18:33:16Z

docs/tutorials/getting_started.rst

+basics of :ref:`inspecting files <getting_started_inspecting>`, :ref:`reading
+<getting_started_reading>` from and :ref:`writing <getting_started_writing>`
+to files, and :ref:`converting <getting_started_converting>` from one format
+to another.  We assume that baseband as well as `NumPy


Nitpick: "baseband" should be capitalized for consistency (though I admit that capitalizing at all is a personal preference).

Since we do it for astropy and numpy in this context, let's do it here too. I have not capitalized it in "baseband module", though...

cczhu · 2018-05-04T18:35:43Z

docs/tutorials/getting_started.rst

+formats.  When opening Mark 4 and Mark 5B files, however, some additional
+arguments may need to be passed (as was the case above for inspecting a Mark
+5B file).  Notes on such features and quirks of individual formats can be
+found in the API entries of their ``open`` functions, and within the


Could also note that file_info can also return missing arguments that are necessary.

cczhu · 2018-05-04T18:35:56Z

docs/tutorials/getting_started.rst

+As shown at the very start, files can be opened with the general
+`baseband.open` function. This will try to determine the file type using
+`~baseband.file_info`, load the corresponding baseband module, and then open
+the file using that modele's master input/output function.


"modele" -> "module"

In particular, decade, kday, and ref_time should be consistent with the file being opened, and nchan with the sample_shape.

Also make sure ``data`` is documented.

mhvk added the enhancement label Apr 4, 2018

mhvk mentioned this pull request Apr 4, 2018

Create an inspection function to determine stream properties #145

Closed

mhvk force-pushed the baseband-open branch 2 times, most recently from 51fca12 to 8bcd303 Compare April 4, 2018 18:54

mhvk force-pushed the baseband-open branch 2 times, most recently from 164eb7a to d753414 Compare April 16, 2018 00:30

mhvk force-pushed the baseband-open branch from d753414 to 77882c6 Compare April 20, 2018 16:38

mhvk changed the title ~~WIP: general opener, using file info.~~ General opener, using file info. Apr 20, 2018

mhvk force-pushed the baseband-open branch from 77882c6 to 6953109 Compare April 20, 2018 18:58

mhvk added this to the 1.1 milestone Apr 20, 2018

mhvk force-pushed the baseband-open branch from 461c807 to 38ccf78 Compare April 24, 2018 16:11

mhvk requested a review from cczhu April 24, 2018 16:21

cczhu reviewed May 3, 2018

View reviewed changes

mhvk added 2 commits May 4, 2018 08:49

Keep track of arguments, and use it in baseband.open.

2fb0e76

Remove superfluous override__dir__ in file info base.

b6d4df4

mhvk force-pushed the baseband-open branch from 38ccf78 to 7baddab Compare May 4, 2018 12:49

cczhu approved these changes May 5, 2018

View reviewed changes

mhvk added 2 commits May 6, 2018 12:40

Test known arguments for consistency.

0ea30fc

In particular, decade, kday, and ref_time should be consistent with the file being opened, and nchan with the sample_shape.

Add documentation for baseband.open

fe4d65a

Also make sure ``data`` is documented.

mhvk force-pushed the baseband-open branch from 7baddab to fe4d65a Compare May 6, 2018 16:41

mhvk merged commit 77cdeee into master May 6, 2018

mhvk deleted the baseband-open branch May 6, 2018 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General opener, using file info. #198

General opener, using file info. #198

mhvk commented Apr 4, 2018

mhvk commented Apr 4, 2018

mhvk commented Apr 16, 2018

mhvk commented Apr 20, 2018

mhvk commented Apr 24, 2018

cczhu left a comment

cczhu May 3, 2018

cczhu May 3, 2018

mhvk May 4, 2018

cczhu May 4, 2018

mhvk May 6, 2018

cczhu left a comment

cczhu May 4, 2018

cczhu May 4, 2018

mhvk May 6, 2018

cczhu May 4, 2018

mhvk May 6, 2018

cczhu May 4, 2018

mhvk May 6, 2018

cczhu May 4, 2018

mhvk May 6, 2018

cczhu May 4, 2018

mhvk May 6, 2018

cczhu May 4, 2018

cczhu May 4, 2018

mhvk May 6, 2018

cczhu May 4, 2018

mhvk May 6, 2018

cczhu May 4, 2018

mhvk May 6, 2018



		@pytest.mark.parametrize('sample', (SAMPLE_M4, SAMPLE_M5B))
		def test_open_missing_args(sample):

		@@ -51,15 +51,17 @@ troubleshooting help and APIs for each.
		Core framework and utilities

General opener, using file info. #198

General opener, using file info. #198

Conversation

mhvk commented Apr 4, 2018

mhvk commented Apr 4, 2018

mhvk commented Apr 16, 2018

mhvk commented Apr 20, 2018

mhvk commented Apr 24, 2018

cczhu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cczhu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment