Rhessi returns database files of multiple months . #2418
Conversation
|
Hello @yashkgp! Thanks for updating the PR.
Comment last updated on February 17, 2018 at 09:54 Hours UTC |
|
Unfortunately, the change breaks a test we have in SunPy. Either this test would need to be changed or removed. Before that however, since I am unsure if this fixes the issue (rhessi is a strange part of SunPy), could you provide a code example and show me the output before and after this fix has been applied? |
|
Here is a code snippet : >>> import sunpy.instr.rhessi as rhessi
>>> fname = rhessi.get_obssumm_file(('2011/04/04', '2011/04/06'))
>>> print (fname)The result was a list , containing the directory of saved file and info() method returned by the urlopen If this doesn't fixes,could you help me out ? |
|
Comparing it current master that does seem to now provide more than one day. The next step is to fix the test that is failing. Then update the documentation where possible about this change. @ehsteve or @Cadair, can you recall why only one day was supported in the first place? @yashkgp What I will say is that this does not seem to close #2382. That issue is about FIDO unable to search for more than one month worth of data. You would have to test that. |
|
I suspect that changing it there means other changes in the file need to be made. |
|
Hey @nabobalis , I have made a commit that solves issue #2382 And the output was Filenames of different months in the time range as list |
|
And for the |
yashkgp
changed the title
|
The first issue is that your changes broke one of the tests and that needs to be addressed. I am unsure we need another function within the code, we seem to have 3 functions for returning rhessi data. We seem to have, Also if the changes mean we can return more than one day, does I am not very familiar with this code, @ehsteve, what do you think about the changes? |
|
|
||
| Parameters | ||
| ---------- | ||
| time_range : str, TimeRange |
There was a problem hiding this comment.
@Cadair is this a standard that we always also accept a string version of a time range?
|
|
||
| Returns | ||
| ------- | ||
| out : list of lists |
There was a problem hiding this comment.
why is this a list of lists? The examples shows only a list.
There was a problem hiding this comment.
Thats a typo in the example ,The function return list of lists with each list containing data for a single month in the given time range . I'll just update it. Here is a code snippet on how this function works :
>>> import sunpy.instr.rhessi as rhessi
>>> rhessi.get_obssum_filename_multiple_months(('2011/04/04', '2011/05/09'))
And the output was
/home/yash_jain/Desktop/sunpy_env/sunpy/sunpy/instr/rhessi.py:100: UserWarning: Currently only support providing data from one whole day. Only data for 2011-05-01 will be returned
_check_one_day(_time_range)
[['https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110404_042.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110405_031.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110406_041.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110407_031.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110408_038.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110409_030.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110410_028.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110411_024.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110412_024.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110413_022.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110414_017.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110415_020.fits', 'http://hessi.ssl.berkeley.edu/hessidata/metadata/catalog/hsi_obssumm_20110416_016.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110417_017.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110418_017.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110419_024.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110420_029.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110421_025.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110422_029.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110423_030.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110424_025.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110425_026.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110426_022.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110427_029.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110428_021.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110429_028.fits'], ['https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110501_021.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110502_023.fits', 'http://hessi.ssl.berkeley.edu/hessidata/metadata/catalog/hsi_obssumm_20110503_027.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110504_032.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110505_028.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110506_031.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110507_024.fits', 'https://hesperia.gsfc.nasa.gov/hessidata/metadata/catalog/hsi_obssumm_20110508_022.fits']]
Filenames of different months in the time range as list
There was a problem hiding this comment.
I think it would be useful for the function to clean this up and make it just a simple list.
There was a problem hiding this comment.
Done !
Could you suggest a name for the method or the current one is fine ?
| """ | ||
| time_range = TimeRange(time_range) | ||
| filenames = [] | ||
| delta = relativedelta(time_range.end, time_range.start) |
There was a problem hiding this comment.
Is there a reason to use this function as opposed to just
delta = time_range.end - time_range.start
There was a problem hiding this comment.
Sorry. I've educated myself using stackoverflow and now I'm wondering if TimeRange should be using that rather than timeldeta since it's so much more useful, @Cadair?
| filenames = [] | ||
| delta = relativedelta(time_range.end, time_range.start) | ||
| date = time_range.start | ||
| if delta.years <= 0 and delta.months <= 0 and time_range.start.month != time_range.end.month : |
There was a problem hiding this comment.
Is this code here just to check that the time_range here is less than one month and which case the user should not be using this function at all, correct?
There was a problem hiding this comment.
Its partially true , Actually get_obssum_filename cannot return filenames spanning over multiple months even when its less than 1 month. (Ex . Time_range(("2011/04/28"),("2011/05/05")) ). Rhessi servers return data of a single month when get_obssum_filename is called ,so this code makes sure that if time_range is less than a month and spans over 2 month then get_obssum_filename should be called 2 times .
|
What above also changing the name of the function to be more explicit?
Would a better name be something like *get_observation_summary_filenames* ?
…On Tue, Feb 13, 2018 at 11:45 AM, Steven Christe ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Overall, I think I would suggest that the original get_obssum_filename be
modified to return multiple files if needed as opposed to creating a new
function.
------------------------------
In sunpy/instr/rhessi.py
<#2418 (comment)>:
> @@ -219,7 +219,55 @@ def get_obssum_filename(time_range):
filenames = dbase_dat.get('filename')[index_number_start:index_number_end]
return [posixpath.join(get_base_url(), 'metadata', 'catalog', filename + 's')
- for filename in filenames]
+ for filename in filenames]
+
+
+def get_obssum_filename_multiple_months(time_range):
+ """
+ Download the RHESSI observing summary data from one of the RHESSI
+ servers, parses it, and returns the name of the obssumm files relevant for
+ the time range.
+
+ Parameters
+ ----------
+ time_range : str, TimeRange
@Cadair <https://github.com/cadair> is this a standard that we always
also accept a string version of a time range?
------------------------------
In sunpy/instr/rhessi.py
<#2418 (comment)>:
> +
+
+def get_obssum_filename_multiple_months(time_range):
+ """
+ Download the RHESSI observing summary data from one of the RHESSI
+ servers, parses it, and returns the name of the obssumm files relevant for
+ the time range.
+
+ Parameters
+ ----------
+ time_range : str, TimeRange
+ A TimeRange or time range compatible string
+
+ Returns
+ -------
+ out : list of lists
why is this a list of lists? The examples shows only a list.
------------------------------
In sunpy/instr/rhessi.py
<#2418 (comment)>:
> + Returns the filenames of the observation summary file for multiple months
+ in the time range
+
+ Examples
+ --------
+ >>> import sunpy.instr.rhessi as rhessi
+ >>> rhessi.get_obssum_filename_multiple_months(('2011/04/04', '2011/04/05')) # doctest: +SKIP
+ ['http://soleil.i4ds.ch/hessidata/metadata/catalog/hsi_obssumm_20110404_042.fits']
+
+ .. note::
+ This API is currently limited to providing data from whole days only.
+
+ """
+ time_range = TimeRange(time_range)
+ filenames = []
+ delta = relativedelta(time_range.end, time_range.start)
Is there a reason to use this function as opposed to just
delta = time_range.end - time_range.start
------------------------------
In sunpy/instr/rhessi.py
<#2418 (comment)>:
> +
+ Examples
+ --------
+ >>> import sunpy.instr.rhessi as rhessi
+ >>> rhessi.get_obssum_filename_multiple_months(('2011/04/04', '2011/04/05')) # doctest: +SKIP
+ ['http://soleil.i4ds.ch/hessidata/metadata/catalog/hsi_obssumm_20110404_042.fits']
+
+ .. note::
+ This API is currently limited to providing data from whole days only.
+
+ """
+ time_range = TimeRange(time_range)
+ filenames = []
+ delta = relativedelta(time_range.end, time_range.start)
+ date = time_range.start
+ if delta.years <= 0 and delta.months <= 0 and time_range.start.month != time_range.end.month :
Is this code here just to check that the time_range here is less than one
month and which case the user should not be using this function at all,
correct?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2418 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA8CF66WyhfhZwuiJ3kP7nFX_iZElQsGks5tUbwlgaJpZM4RmSPr>
.
|
|
@wafels, FYI, I used that short-hand (obssumm) because it is a term often used on the RHESSI side though that should not stop us from changing it. |
Fixes #2382
Made a new function that returns Rhessi files for multiple months