Skip to content
This repository

Fix from table #24

Merged
merged 7 commits into from over 1 year ago

3 participants

Wolfgang Kerzendorf Adam Ginsburg Hans Moritz Günther
Wolfgang Kerzendorf
Collaborator

Fixed up from_table-classmethod to add documentation and masks and flags. This should be merged after #23.

Wolfgang Kerzendorf
Collaborator

@keflavich @hamogu any comments? if not will merge in 6 hours

Hans Moritz Günther hamogu commented on the diff
specutils/spectrum1d.py
((44 lines not shown))
82 107 flux = table[flux_column]
83   - disp = table[dispersion_column]
  108 + dispersion = table[dispersion_column]
84 109
85 110 if uncertainty_column is not None:
86 111 uncertainty = table[uncertainty_column]
2
Hans Moritz Günther Collaborator
hamogu added a note

Should we check that flux.units and uncertainty.units are the same? Spectrum1D expects them to be the same (which is a sensible design decision, I think).
I can easility imagine cases, where the input table has e.g. fluxes in 10^-7 erg/s/cm^2 and uncertainities in 10^-9 erg/s/cm. This is not a place to convert the units, but we could issue a warning if they don't match.

Wolfgang Kerzendorf Collaborator

@hamogu done! Check it out and if you're happy we can merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
specutils/spectrum1d.py
((47 lines not shown))
84 109
85 110 if uncertainty_column is not None:
86 111 uncertainty = table[uncertainty_column]
  112 + if uncertainty.unit is not flux.unit:
2
Adam Ginsburg Collaborator
keflavich added a note

Is this the right check? i.e., are uncertainty.unit and flux.unit the same object? Usually I'd think == is more appropriate, but not sure.

Hans Moritz Günther Collaborator
hamogu added a note

+1 to not ... == ... , or more to !=

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
specutils/spectrum1d.py
((53 lines not shown))
87 115 else:
88 116 uncertainty = None
89 117
90   - return cls.from_array(flux=flux.data, disp=disp.data, uncertainty=uncertainty, dispersion_unit=disp.units, unit=flux.units)
  118 + if isinstance(flag_columns, basestring):
  119 + flags = table[flag_columns]
  120 + elif misc.isiterable(flag_columns):
  121 + flags = FlagCollection(shape=flux.shape)
  122 + for flag_column in flag_columns:
  123 + flags[flag_column] = table[flag_column]
  124 + else:
  125 + raise ValueError('flag_columns should either be a string or a list of strings')
3
Adam Ginsburg Collaborator
keflavich added a note

a list -> an iterable

Hans Moritz Günther Collaborator
hamogu added a note

@keflavich While you are certainly right, I kind of like the simpler language of a list. In 90% of the use cases it probably is a list of strings and for a python beginner the term "an iterable" is a hard nut to crack.

Adam Ginsburg Collaborator
keflavich added a note

@hamogu I generally agree with you. Maybe for precision's sake, we could say a list (or an iterable)? It's a little more verbose, but I think it would be good to know as a user if a tuple or array is acceptable (saves me from doing arr.tolist() thinking that only lists are OK)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
specutils/spectrum1d.py
((53 lines not shown))
87 115 else:
88 116 uncertainty = None
89 117
90   - return cls.from_array(flux=flux.data, disp=disp.data, uncertainty=uncertainty, dispersion_unit=disp.units, unit=flux.units)
  118 + if isinstance(flag_columns, basestring):
  119 + flags = table[flag_columns]
  120 + elif misc.isiterable(flag_columns):
  121 + flags = FlagCollection(shape=flux.shape)
  122 + for flag_column in flag_columns:
  123 + flags[flag_column] = table[flag_column]
  124 + else:
  125 + raise ValueError('flag_columns should either be a string or a list of strings')
  126 +
  127 + return cls.from_array(flux=flux.data, dispersion=dispersion.data, uncertainty=uncertainty,
  128 + dispersion_unit=dispersion.units, unit=flux.units, mask=table.mask, flags=flags)
1
Adam Ginsburg Collaborator
keflavich added a note

Line wrap? I can't see the end of this line on github, so I assume its >80 chars. (others below too)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Adam Ginsburg
Collaborator

Only a few minor changes needed, then it can be merged.

Wolfgang Kerzendorf
Collaborator

@keflavich @hamogu anything else - otherwise will merge in 6 hours.

Wolfgang Kerzendorf wkerzendorf merged commit d00f357 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 58 additions and 20 deletions. Show diff stats Hide diff stats

  1. +58 20 specutils/spectrum1d.py
78 specutils/spectrum1d.py
@@ -6,7 +6,9 @@
6 6 __all__ = ['Spectrum1D', 'spec_operation']
7 7
8 8 from astropy import log
9   -from astropy.nddata import NDData
  9 +from astropy.nddata import NDData, FlagCollection
  10 +
  11 +from astropy.utils import misc
10 12
11 13 from .wcs import Spectrum1DLookupWCS, Spectrum1DLinearWCS
12 14
@@ -22,13 +24,13 @@ class Spectrum1D(NDData):
22 24
23 25
24 26 @classmethod
25   - def from_array(cls, disp, flux, dispersion_unit=None, uncertainty=None, mask=None, flags=None, meta=None,
  27 + def from_array(cls, dispersion, flux, dispersion_unit=None, uncertainty=None, mask=None, flags=None, meta=None,
26 28 unit=None, copy=True):
27 29 """Initialize `Spectrum1D`-object from two `numpy.ndarray` objects
28 30
29 31 Parameters:
30 32 -----------
31   - disp : `~numpy.ndarray`
  33 + dispersion : `~numpy.ndarray`
32 34 The dispersion for the Spectrum (i.e. an array of wavelength points).
33 35
34 36 flux : `~numpy.ndarray`
@@ -67,27 +69,64 @@ def from_array(cls, disp, flux, dispersion_unit=None, uncertainty=None, mask=Non
67 69 Raises
68 70 ------
69 71 ValueError
70   - If the `disp` and `flux` arrays cannot be broadcast (e.g. their shapes
  72 + If the `dispersion` and `flux` arrays cannot be broadcast (e.g. their shapes
71 73 do not match), or the input arrays are not one dimensional.
72 74
73 75 """
74 76
75   - if disp.ndim != 1 or disp.shape != flux.shape:
76   - raise ValueError("disp and flux need to be one-dimensional Numpy arrays with the same shape")
77   - spec_wcs = Spectrum1DLookupWCS(disp, unit=dispersion_unit)
  77 + if dispersion.ndim != 1 or dispersion.shape != flux.shape:
  78 + raise ValueError("dispersion and flux need to be one-dimensional Numpy arrays with the same shape")
  79 + spec_wcs = Spectrum1DLookupWCS(dispersion, unit=dispersion_unit)
78 80 return cls(data=flux, wcs=spec_wcs, unit=unit)
79 81
80 82 @classmethod
81   - def from_table(cls, table, mask=None, dispersion_column='disp', flux_column='flux', uncertainty_column=None):
  83 + def from_table(cls, table, dispersion_column='dispersion', flux_column='flux', uncertainty_column=None,
  84 + flag_columns=None):
  85 + """
  86 + Initializes a `Spectrum1D`-object from an `~astropy.table.Table` object
  87 +
  88 + Parameters
  89 + ----------
  90 +
  91 + table : ~astropy.table.Table object
  92 +
  93 + dispersion_column : str, optional
  94 + name of the dispersion column. default is 'dispersion'
  95 +
  96 + flux_column : str, optional
  97 + name of the flux column. default is 'flux'
  98 +
  99 + uncertainty_column : str, optional
  100 + name of the uncertainty column. If set to None uncertainty is set to None. default is None
  101 +
  102 + flag_columns : str or list, optional
  103 + name or names of flag columns. If multiple names are supplied a ~astropy.nddata.FlagCollection will be built.
  104 + default is None
  105 + """
  106 +
82 107 flux = table[flux_column]
83   - disp = table[dispersion_column]
  108 + dispersion = table[dispersion_column]
84 109
85 110 if uncertainty_column is not None:
86 111 uncertainty = table[uncertainty_column]
  112 + if uncertainty.unit != flux.unit:
  113 + log.warning('"uncertainty"-column and "flux"-column do not share the units (%s vs %s) ',
  114 + uncertainty.unit, flux.unit)
87 115 else:
88 116 uncertainty = None
89 117
90   - return cls.from_array(flux=flux.data, disp=disp.data, uncertainty=uncertainty, dispersion_unit=disp.units, unit=flux.units)
  118 + if isinstance(flag_columns, basestring):
  119 + flags = table[flag_columns]
  120 + elif misc.isiterable(flag_columns):
  121 + flags = FlagCollection(shape=flux.shape)
  122 + for flag_column in flag_columns:
  123 + flags[flag_column] = table[flag_column]
  124 + else:
  125 + raise ValueError('flag_columns should either be a string or a list (or iterable) of strings')
  126 +
  127 + return cls.from_array(flux=flux.data, dispersion=dispersion.data,
  128 + uncertainty=uncertainty, dispersion_unit=dispersion.units,
  129 + unit=flux.units, mask=table.mask, flags=flags)
91 130
92 131
93 132
@@ -102,7 +141,7 @@ def from_ascii(cls, filename, uncertainty=None, mask=None, dtype=np.float, comme
102 141 if raw_data.shape[1] != 2:
103 142 raise ValueError('data contained in filename must have exactly two columns')
104 143
105   - return cls.from_array(disp=raw_data[:,0], flux=raw_data[:,1], uncertainty=uncertainty, mask=mask)
  144 + return cls.from_array(dispersion=raw_data[:,0], flux=raw_data[:,1], uncertainty=uncertainty, mask=mask)
106 145
107 146 @classmethod
108 147 def from_fits(cls, filename, uncertainty=None):
@@ -121,7 +160,7 @@ def flux_setter(self, flux):
121 160 self.data = flux
122 161
123 162 @property
124   - def disp(self):
  163 + def dispersion(self):
125 164 #returning the disp
126 165 if not hasattr(self.wcs, 'lookup_table'):
127 166 self.wcs.create_lookup_table(np.arange(len(self.flux)))
@@ -137,7 +176,7 @@ def flux_unit(self):
137 176 return self.unit
138 177
139 178
140   - def interpolate(self, new_disp, kind='linear', bounds_error=True, fill_value=np.nan):
  179 + def interpolate(self, new_dispersion, kind='linear', bounds_error=True, fill_value=np.nan):
141 180 """Interpolates onto a new wavelength grid and returns a new `Spectrum1D`-object.
142 181
143 182 Parameters
@@ -183,13 +222,13 @@ def interpolate(self, new_disp, kind='linear', bounds_error=True, fill_value=np.
183 222 " interpolate to new dispersion map without this"+
184 223 " (need scipy.interpolate.interp1d)")
185 224
186   - spectrum_interp = interpolate.interp1d(self.disp,
  225 + spectrum_interp = interpolate.interp1d(self.dispersion,
187 226 self.flux,
188 227 kind=kind,
189 228 bounds_error=bounds_error,
190 229 fill_value=fill_value)
191 230
192   - new_flux = spectrum_interp(new_disp)
  231 + new_flux = spectrum_interp(new_dispersion)
193 232
194 233 # We need to perform error calculation for the new dispersion map
195 234 if self.uncertainty is None:
@@ -198,7 +237,7 @@ def interpolate(self, new_disp, kind='linear', bounds_error=True, fill_value=np.
198 237 # After having a short think about it, it seems reasonable to me only to
199 238 # take the nearest uncertainty for each interpolated dispersion point
200 239
201   - new_uncertainty = interpolate.interp1d(self.disp,
  240 + new_uncertainty = interpolate.interp1d(self.dispersion,
202 241 self.flux,
203 242 kind=1, # Nearest
204 243 bounds_error=bounds_error,
@@ -208,7 +247,7 @@ def interpolate(self, new_disp, kind='linear', bounds_error=True, fill_value=np.
208 247 if self.mask is None:
209 248 new_mask = None
210 249 else:
211   - new_mask = interpolate.interp1d(self.disp,
  250 + new_mask = interpolate.interp1d(self.dispersion,
212 251 self.flux,
213 252 kind=1, # Nearest
214 253 bounds_error=bounds_error,
@@ -217,7 +256,7 @@ def interpolate(self, new_disp, kind='linear', bounds_error=True, fill_value=np.
217 256 # As for flags it is not entirely clear to me what the best behaviour is
218 257 # In the face of uncertainty, for the time being, I am discarding flags
219 258
220   - return self.__class__.from_array(new_disp,
  259 + return self.__class__.from_array(new_dispersion,
221 260 new_flux,
222 261 uncertainty=new_uncertainty,
223 262 mask=new_mask,
@@ -296,5 +335,4 @@ def slice_index(self, start=None, stop=None):
296 335 # reasonably) assuming that __slice__ will be a NDData base function
297 336 # which we will inherit.
298 337 raise NotImplemented('Will presumeably implemented in core NDDATA')
299   - return self.__slice__(start_index, stop_index)
300   -
  338 +

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.