condense to a single read per file type with handling for partial reads #393
Conversation
pyuvdata/miriad.py
Outdated
| ant_str: A string containing information about what kinds of visibility data | ||
| to read-in. Can be 'auto', 'cross', 'all'. Cannot provide ant_str if | ||
| antenna_nums and/or bls is not None. | ||
| polarizations: List of polarization integers or strings to read-in. | ||
| Ex: ['xx', 'yy', ...] | ||
| time_range: len-2 list containing min and max range of times (Julian Date) to read-in. | ||
| Ex: [2458115.20, 2458115.40] | ||
| read_data: Read in the visibility and flag data. If set to false, | ||
| only the basic header info and metadata (if read_metadata is True) |
There was a problem hiding this comment.
Is there a separate read_metadata flag? It seems like it's not a kwarg for this function.
There was a problem hiding this comment.
read_metadata is only a thing for uvfits because uvfits essentially has 3 levels of things to read: header info, metadata and data, so you need two keyword arguments to tell it what to read beyond the header. Miriad only has 2 levels: metadata & data, so it only needs one keyword argument (if you call it at all it reads the metadata).
There was a problem hiding this comment.
Yes, that makes sense. Sorry for not being clear, I was just saying that the docstring has a reference to the read_metadata kwarg, so it probably needs to be clarified.
There was a problem hiding this comment.
Ah! Thanks for catching that and I'm sorry I didn't pay enough attention!
pyuvdata/uvdata.py
Outdated
| # work out what function should be called depending on what's | ||
| # already defined on the object | ||
| if self.freq_array is not None: | ||
| hdr_read = True |
There was a problem hiding this comment.
100% a style issue: in my head, I read these variables as "should we read in the data?", not "is the data already loaded?". Calling the variables something like hdr_loaded might make the meaning more clear to the casual reader, but I'm also totally fine keeping it the way it is.
There was a problem hiding this comment.
I agree, I'll change the names as you suggest.
pyuvdata/uvdata.py
Outdated
| else: | ||
| data_read = False | ||
|
|
||
| if not read_data and not read_metadata: |
There was a problem hiding this comment.
A very complicated dance of logic! The in-line explanations are very helpful :)
There was a problem hiding this comment.
good! They helped me write the code :)
pyuvdata/uvdata.py
Outdated
| @@ -1719,6 +1683,21 @@ def read_miriad(self, filepath, correct_lat_lon=True, run_check=True, | |||
| Ex: ['xx', 'yy', ...] | |||
| time_range: len-2 list containing min and max range of times (Julian Date) to read-in. | |||
| Ex: [2458115.20, 2458115.40] | |||
| read_data: Read in the visibility and flag data. If set to false, | |||
| only the basic header info and metadata (if read_metadata is True) | |||
There was a problem hiding this comment.
Same as above, no read_metadata kwarg.
There was a problem hiding this comment.
Same reasoning as above, your HDF5 files only seem to have 2 levels.
bhazelton
force-pushed
the
streamline_read_funcs
branch
from
1cda974
to
d8a5a71
Compare
|
So if I'm not mistaken, this eliminates the |
|
Looks good to me! Feel free to merge. |
This PR unifies the read interfaces as discussed in telecons.
@plaplant can you review this?
@nkern I'd also like to make sure it works for you
Fixes #389
Fixes #386