added function to truncate by index and spectral window #8
Conversation
|
Hi @abit2. What's the reasoning for changing the |
|
Hi @abit2. I've been giving this some more thought. I think what we want is a truncate function, similar to what TimeSeries has. Perhaps we also want something like the sel function in xarray.dataset. But I think trying to get all the functionality we want in one API is asked too much. I'll write up an issue describing what we need so you can start another PR for the truncate function. Once that's done we can come back to a simpler version of this sel function than I initially described. |
|
But before, moving on from this PR, I think you should restore |
|
Here is the link to the issue. Let me know if you have questions and we can discuss it next week. |
| return "<iris.IRISRaster instance\nOBS ID: {0}\n".format(self.meta["observation ID"]) + \ | ||
| "OBS Description: {0}\n".format(self.meta["observation description"]) + \ | ||
| "OBS period: {0} -- {1}\n".format(self.meta["observation start"], self.meta["observation end"]) + \ | ||
| "Instance period: {0} -- {1}\n".format(self.data[spectral_window].time.values[0], |
There was a problem hiding this comment.
if there is only one element in .data[spectral_window].time.values then using 0 th index gives an error
There was a problem hiding this comment.
I think as above, the result will be an array of length 1. So giving the using the 0th index will work. If this is not the case, it should be. We don't want the type of an attribute to change just because the length changes. But again, it would good if you could confirm this.
| @@ -229,17 +229,29 @@ def __init__(self, filenames, spectral_windows="All"): | |||
|
|
|||
| def __repr__(self): | |||
| spectral_window = self.spectral_windows["name"][0] | |||
| spectral_windows_info = "".join( | |||
| ["\n {0}\n (raster axis: {1}, slit axis: {2}, spectral axis: {3})".format( | |||
| name, len(self.data[name].raster_axis), len(self.data[name].slit_axis), | |||
There was a problem hiding this comment.
suppose someone studies only one raster_axis so he/she truncates by index then length of an integer gives an error.
There was a problem hiding this comment.
I believe that if there is only one element along the raster_axis, then self.data[name].raster_axis will be an array of length 1, not an integer. If this is the case, then it should still work as written.
|
@DanRyanIrish can you please look into the PR when you have time. |
| @@ -229,17 +229,29 @@ def __init__(self, filenames, spectral_windows="All"): | |||
|
|
|||
| def __repr__(self): | |||
| spectral_window = self.spectral_windows["name"][0] | |||
| spectral_windows_info = "".join( | |||
| ["\n {0}\n (raster axis: {1}, slit axis: {2}, spectral axis: {3})".format( | |||
| name, len(self.data[name].raster_axis), len(self.data[name].slit_axis), | |||
There was a problem hiding this comment.
I believe that if there is only one element along the raster_axis, then self.data[name].raster_axis will be an array of length 1, not an integer. If this is the case, then it should still work as written.
irispy/spectrograph.py
Outdated
| @@ -15,9 +15,9 @@ | |||
| from astropy import constants | |||
| from scipy import interpolate | |||
| from sunpy.time import parse_time | |||
|
|
|||
| from sunpy.net.vso import attrs as a | |||
There was a problem hiding this comment.
I think we should have more descriptive variable names than a. But attrs is also too generic. Perhaps it would be better to do from sunpy.net import vso.attrs.
| return "<iris.IRISRaster instance\nOBS ID: {0}\n".format(self.meta["observation ID"]) + \ | ||
| "OBS Description: {0}\n".format(self.meta["observation description"]) + \ | ||
| "OBS period: {0} -- {1}\n".format(self.meta["observation start"], self.meta["observation end"]) + \ | ||
| "Instance period: {0} -- {1}\n".format(self.data[spectral_window].time.values[0], |
There was a problem hiding this comment.
I think as above, the result will be an array of length 1. So giving the using the 0th index will work. If this is not the case, it should be. We don't want the type of an attribute to change just because the length changes. But again, it would good if you could confirm this.
|
I'll take a closer look at the new parts of your code during the week. Thanks again for the contribution. |
It is the initial commit for IRISRaster object method with which object can now be truncated by index of time, raster_position, slit_axis and spectral window at the moment.
let me know if there is any need for more by index method.
Made changes to the repr as it was not able to show the changes in the data so made it somewhat dynamic for now, not sure if it will work for every change may need more work.
More work will be done on it in this branch:).
@DanRyanIrish