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

Fixes XRSTimeSeries bug with older (around 1986) XRS fits files #3081

Merged
merged 5 commits into from Apr 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/3081.bugfix.rst
@@ -0,0 +1 @@
Fixes bug when creating a timeseries from a URL and bug when creating a TimeSeries from older GOES/XRS fits files.
12 changes: 5 additions & 7 deletions sunpy/timeseries/sources/goes.py
Expand Up @@ -112,8 +112,8 @@ def peek(self, title="GOES Xray Flux"):
figure.show()

# ToDo: is this part of the DL pipeline? If so delete.
@classmethod
def _get_goes_sat_num(self, start, end):
@staticmethod
def _get_goes_sat_num(start, end):
"""Parses the query time to determine which GOES satellite to use."""

goes_operational = {
Expand All @@ -133,10 +133,8 @@ def _get_goes_sat_num(self, start, end):

sat_list = []
for sat_num in goes_operational:
if ((start >= goes_operational[sat_num].start and
start <= goes_operational[sat_num].end and
(end >= goes_operational[sat_num].start and
end <= goes_operational[sat_num].end))):
if (goes_operational[sat_num].start <= start <= goes_operational[sat_num].end and
goes_operational[sat_num].start <= end <= goes_operational[sat_num].end):
# if true then the satellite with sat_num is available
sat_list.append(sat_num)

Expand Down Expand Up @@ -168,7 +166,7 @@ def _parse_hdus(cls, hdulist):
xrsa = hdulist[2].data['FLUX'][0][:, 1]
seconds_from_start = hdulist[2].data['TIME'][0]
elif 1 <= len(hdulist) <= 3:
start_time = parse_time(header['TIMEZERO'])
start_time = parse_time(header['TIMEZERO'], format='utime')
seconds_from_start = hdulist[0].data[0]
xrsb = hdulist[0].data[1]
xrsa = hdulist[0].data[2]
Expand Down
15 changes: 15 additions & 0 deletions sunpy/timeseries/tests/test_timeseries_factory.py
Expand Up @@ -178,6 +178,21 @@ def test_noaa_pre(self):
ts_noaa_pre = sunpy.timeseries.TimeSeries(noaa_pre_filepath, source='NOAAPredictIndices')
assert isinstance(ts_noaa_pre, sunpy.timeseries.sources.noaa.NOAAPredictIndicesTimeSeries)

# ==============================================================================
# Remote Sources Tests
# ==============================================================================

@pytest.mark.remote_data
def test_goes_remote(self):
# Older format file
goes = sunpy.timeseries.TimeSeries(
'https://umbra.nascom.nasa.gov/goes/fits/1986/go06860129.fits')
assert isinstance(goes, sunpy.timeseries.sources.goes.XRSTimeSeries)
# Newer format
goes = sunpy.timeseries.TimeSeries(
'https://umbra.nascom.nasa.gov/goes/fits/2018/go1520180626.fits')
assert isinstance(goes, sunpy.timeseries.sources.goes.XRSTimeSeries)

#==============================================================================
# Manual TimeSeries Tests
#==============================================================================
Expand Down
76 changes: 34 additions & 42 deletions sunpy/timeseries/timeseries_factory.py
Expand Up @@ -103,7 +103,8 @@ class TimeSeriesFactory(BasicRegistrationFactory):
... 'file1.fits', url, 'eit_*.fits') # doctest: +SKIP
"""

def _read_file(self, fname, **kwargs):
@staticmethod
def _read_file(fname, **kwargs):
"""
Test reading a file with sunpy.io for automatic source detection.

Expand Down Expand Up @@ -141,7 +142,8 @@ def _read_file(self, fname, **kwargs):
else:
return False, fname

def _validate_meta(self, meta):
@staticmethod
def _validate_meta(meta):
"""
Validate a meta argument for use as metadata.
Currently only validates by class.
Expand All @@ -155,7 +157,8 @@ def _validate_meta(self, meta):
else:
return False

def _validate_units(self, units):
@staticmethod
def _validate_units(units):
"""
Validates the astropy unit-information associated with a TimeSeries.
Should be a dictionary of some form (but not MetaDict) with only
Expand All @@ -177,7 +180,8 @@ def _validate_units(self, units):
# Passed all the tests
return result

def _from_table(self, t):
@staticmethod
def _from_table(t):
"""
Extract the data, metadata and units from an astropy table for use in
constructing a TimeSeries.
Expand All @@ -200,9 +204,7 @@ def _from_table(self, t):
# Check if another column is defined as the index/primary_key
if table.primary_key:
# Check there is only one primary_key/index column
if len(table.primary_key) == 1:
table.primary_key[0]
else:
if len(table.primary_key) != 1:
raise ValueError("Invalid input Table, TimeSeries doesn't support conversion"
" of tables with more then one index column.")

Expand Down Expand Up @@ -256,9 +258,6 @@ def _parse_args(self, *args, **kwargs):
already_timeseries = list()
filepaths = list()

# Take source kwarg if defined
source = kwargs.get('source', None)

# Account for nested lists of items. Simply outputs a single list of
# items, nested lists are expanded to element level.
args = expand_list(args)
Expand All @@ -269,8 +268,7 @@ def _parse_args(self, *args, **kwargs):
arg = args[i]

# Data-header pair in a tuple
if (isinstance(arg, (np.ndarray, Table, pd.DataFrame))):
# and self._validate_meta(args[i+1])):
if isinstance(arg, (np.ndarray, Table, pd.DataFrame)):
# Assume a Pandas Dataframe is given
data = arg
units = OrderedDict()
Expand All @@ -286,7 +284,7 @@ def _parse_args(self, *args, **kwargs):

# If there are 1 or 2 more arguments:
for _ in range(2):
if (len(args) > i+1):
if len(args) > i+1:
# If that next argument isn't data but is metaddata or units:
if not isinstance(args[i+1], (np.ndarray, Table, pd.DataFrame)):
if self._validate_units(args[i+1]):
Expand All @@ -308,13 +306,8 @@ def _parse_args(self, *args, **kwargs):
os.path.isfile(os.path.expanduser(arg))):

path = os.path.expanduser(arg)

read, result = self._read_file(path, **kwargs)

if read:
data_header_pairs.append(result)
else:
filepaths.append(result)
result = self._read_file(path, **kwargs)
data_header_pairs, filepaths = _apply_result(data_header_pairs, filepaths, result)

# Directory
elif (isinstance(arg, str) and
Expand All @@ -325,26 +318,20 @@ def _parse_args(self, *args, **kwargs):
for afile in files:
# returns a boolean telling us if it were read and either a
# tuple or the original filepath for reading by a source
read, result = self._read_file(afile, **kwargs)
if read:
data_header_pairs.append(result)
else:
filepaths.append(result)
result = self._read_file(afile, **kwargs)
data_header_pairs, filepaths = _apply_result(data_header_pairs, filepaths,
result)

# Glob
elif (isinstance(arg, str) and '*' in arg):
elif isinstance(arg, str) and '*' in arg:

files = glob.glob(os.path.expanduser(arg))

for afile in files:
# data_header_unit_tuples += self._read_file(afile, **kwargs)
# returns a boolean telling us if it were read and either a
# tuple or the original filepath for reading by a source
read, result = self._read_file(afile, **kwargs)
if read:
data_header_pairs.append(result)
else:
filepaths.append(result)
result = self._read_file(afile, **kwargs)
data_header_pairs, filepaths = _apply_result(data_header_pairs, filepaths,
result)

# Already a TimeSeries
elif isinstance(arg, GenericTimeSeries):
Expand All @@ -355,12 +342,9 @@ def _parse_args(self, *args, **kwargs):
_is_url(arg)):
url = arg
path = download_file(url, get_and_create_download_dir())
pairs = self._read_file(path, **kwargs)
# data_header_pairs += pairs
filepaths.append(pairs[1])

result = self._read_file(path, **kwargs)
data_header_pairs, filepaths = _apply_result(data_header_pairs, filepaths, result)
else:
# raise ValueError("File not found or invalid input")
raise NoMatchError("File not found or invalid input")
i += 1

Expand Down Expand Up @@ -402,14 +386,13 @@ def __call__(self, *args, **kwargs):
for filepath in filepaths:
try:
new_ts = self._check_registered_widgets(filepath=filepath, **kwargs)
new_timeseries.append(new_ts)
except (NoMatchError, MultipleMatchError, ValidationFunctionError):
if not silence_errors:
raise
except Exception:
raise

new_timeseries.append(new_ts)

# data_header_pairs is a list of HDUs as read by sunpy.io
# For each set of HDus find the matching class and read the
# data_header_unit_tuples by calling the _parse_hdus method
Expand Down Expand Up @@ -456,14 +439,13 @@ def __call__(self, *args, **kwargs):
try:
new_ts = self._check_registered_widgets(data=data, meta=meta,
units=units, **kwargs)
new_timeseries.append(new_ts)
except (NoMatchError, MultipleMatchError, ValidationFunctionError):
if not silence_errors:
raise
except Exception:
raise

new_timeseries.append(new_ts)

new_timeseries += already_timeseries

# Concatenate the timeseries into one if specified.
Expand Down Expand Up @@ -531,6 +513,16 @@ def _check_registered_widgets(self, **kwargs):
return WidgetType(data, meta, units, **kwargs)


def _apply_result(data_header_pairs, filepaths, result):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No really sure about this myself just the code was repeated 3/4 time in quick succession ...

Copy link
Member

Choose a reason for hiding this comment

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

not sure about the name, but separating it out seems to make sense.

read, result = result
if read:
data_header_pairs.append(result)
else:
filepaths.append(result)

return data_header_pairs, filepaths


def _is_url(arg):
try:
urlopen(arg)
Expand Down