Download from fido search result implemented #1877

Open
wants to merge 56 commits into
from

Conversation

4 participants
@Punyaslok
Member

Punyaslok commented Aug 11, 2016

This PR also contains the commits in #1830 and #1785 and #1856

@Cadair

@Cadair Cadair changed the title from Download from fido search result implemented to [unidown] Download from fido search result implemented Aug 11, 2016

@Cadair Cadair added the unidown label Sep 15, 2016

@dpshelio dpshelio changed the base branch from unidown to master Jan 11, 2017

@Cadair Cadair changed the title from [unidown] Download from fido search result implemented to Download from fido search result implemented Jan 11, 2017

@stale stale bot added the Stale label Jun 11, 2017

@stale stale bot closed this Jul 12, 2017

@Cadair Cadair self-assigned this Jul 12, 2017

@Punyaslok Punyaslok reopened this Aug 5, 2017

@pep8speaks

This comment has been minimized.

Show comment
Hide comment
@pep8speaks

pep8speaks Aug 5, 2017

Hello @Punyaslok! Thanks for updating the PR.

Line 161:101: E501 line too long (104 > 100 characters)
Line 164:101: E501 line too long (103 > 100 characters)
Line 465:101: E501 line too long (101 > 100 characters)
Line 473:21: E128 continuation line under-indented for visual indent
Line 680:101: E501 line too long (101 > 100 characters)

Line 430:9: E265 block comment should start with '# '
Line 432:9: E265 block comment should start with '# '
Line 664:101: E501 line too long (103 > 100 characters)
Line 708:101: E501 line too long (101 > 100 characters)
Line 727:101: E501 line too long (118 > 100 characters)
Line 731:29: E128 continuation line under-indented for visual indent
Line 732:14: E114 indentation is not a multiple of four (comment)
Line 733:14: E114 indentation is not a multiple of four (comment)
Line 736:101: E501 line too long (120 > 100 characters)
Line 740:33: E128 continuation line under-indented for visual indent

Line 81:5: E265 block comment should start with '# '
Line 85:8: E128 continuation line under-indented for visual indent
Line 88:9: E128 continuation line under-indented for visual indent
Line 91:9: E128 continuation line under-indented for visual indent
Line 94:9: E128 continuation line under-indented for visual indent
Line 97:8: E128 continuation line under-indented for visual indent
Line 100:10: E128 continuation line under-indented for visual indent
Line 103:9: E128 continuation line under-indented for visual indent
Line 106:9: E128 continuation line under-indented for visual indent
Line 580:5: E125 continuation line with same indent as next logical line
Line 580:5: E128 continuation line under-indented for visual indent
Line 581:1: W293 blank line contains whitespace
Line 705:50: E231 missing whitespace after ','
Line 715:9: E128 continuation line under-indented for visual indent
Line 725:9: E128 continuation line under-indented for visual indent
Line 748:50: E231 missing whitespace after ','
Line 758:9: E128 continuation line under-indented for visual indent
Line 768:9: E128 continuation line under-indented for visual indent

Line 382:47: E128 continuation line under-indented for visual indent

Line 27:8: E221 multiple spaces before operator
Line 29:1: E302 expected 2 blank lines, found 1
Line 150:9: E265 block comment should start with '# '

Comment last updated on May 03, 2018 at 19:58 Hours UTC

pep8speaks commented Aug 5, 2017

Hello @Punyaslok! Thanks for updating the PR.

Line 161:101: E501 line too long (104 > 100 characters)
Line 164:101: E501 line too long (103 > 100 characters)
Line 465:101: E501 line too long (101 > 100 characters)
Line 473:21: E128 continuation line under-indented for visual indent
Line 680:101: E501 line too long (101 > 100 characters)

Line 430:9: E265 block comment should start with '# '
Line 432:9: E265 block comment should start with '# '
Line 664:101: E501 line too long (103 > 100 characters)
Line 708:101: E501 line too long (101 > 100 characters)
Line 727:101: E501 line too long (118 > 100 characters)
Line 731:29: E128 continuation line under-indented for visual indent
Line 732:14: E114 indentation is not a multiple of four (comment)
Line 733:14: E114 indentation is not a multiple of four (comment)
Line 736:101: E501 line too long (120 > 100 characters)
Line 740:33: E128 continuation line under-indented for visual indent

Line 81:5: E265 block comment should start with '# '
Line 85:8: E128 continuation line under-indented for visual indent
Line 88:9: E128 continuation line under-indented for visual indent
Line 91:9: E128 continuation line under-indented for visual indent
Line 94:9: E128 continuation line under-indented for visual indent
Line 97:8: E128 continuation line under-indented for visual indent
Line 100:10: E128 continuation line under-indented for visual indent
Line 103:9: E128 continuation line under-indented for visual indent
Line 106:9: E128 continuation line under-indented for visual indent
Line 580:5: E125 continuation line with same indent as next logical line
Line 580:5: E128 continuation line under-indented for visual indent
Line 581:1: W293 blank line contains whitespace
Line 705:50: E231 missing whitespace after ','
Line 715:9: E128 continuation line under-indented for visual indent
Line 725:9: E128 continuation line under-indented for visual indent
Line 748:50: E231 missing whitespace after ','
Line 758:9: E128 continuation line under-indented for visual indent
Line 768:9: E128 continuation line under-indented for visual indent

Line 382:47: E128 continuation line under-indented for visual indent

Line 27:8: E221 multiple spaces before operator
Line 29:1: E302 expected 2 blank lines, found 1
Line 150:9: E265 block comment should start with '# '

Comment last updated on May 03, 2018 at 19:58 Hours UTC
@Cadair

It would be good to move Database.fetch over to using Fido.

@Cadair Cadair added this to Nice to Haves in SunPy 0.8 Aug 10, 2017

@nabobalis

This comment has been minimized.

Show comment
Hide comment
@nabobalis

nabobalis Aug 10, 2017

Contributor

Here is a non suspicious patch in a zip.
patch.zip

Will not fix the online tests though.

Contributor

nabobalis commented Aug 10, 2017

Here is a non suspicious patch in a zip.
patch.zip

Will not fix the online tests though.

@nabobalis

This comment has been minimized.

Show comment
Hide comment
@nabobalis

nabobalis Aug 11, 2017

Contributor

Looks like a real test failure.

    def test_entries_from_file_time_string_parse_format():
    
        with pytest.raises(ValueError):
            # Error should be  raised because of the date format in GOES_DATA
>           entries = list(entries_from_file(GOES_DATA))
E           Failed: DID NOT RAISE <class 'ValueError'>
Contributor

nabobalis commented Aug 11, 2017

Looks like a real test failure.

    def test_entries_from_file_time_string_parse_format():
    
        with pytest.raises(ValueError):
            # Error should be  raised because of the date format in GOES_DATA
>           entries = list(entries_from_file(GOES_DATA))
E           Failed: DID NOT RAISE <class 'ValueError'>

@nabobalis nabobalis added this to Non-Blocking Tasks in SunPy 0.9 Aug 12, 2017

Punyaslok added some commits Jun 14, 2016

Added an else condition. Previously if path was not none and {file} w…
…as in path, then fname was not assigned any value as both if and elif were skipped.
Try except added to the instrument_name assignment because for goes, …
…it was erroring. Commented out the else raise part because it was halting the making of DB entries for as soon as the else was executed.

@Punyaslok Punyaslok reopened this May 2, 2018

@sunpy sunpy deleted a comment from stale bot May 2, 2018

@sunpy sunpy deleted a comment from stale bot May 2, 2018

@sunpy sunpy deleted a comment from stale bot May 2, 2018

+
+ >>> from sunpy.net import Fido, attrs as a # doctest: +REMOTE_DATA
+ >>> search_result = Fido.search(a.Time("2012/1/1", "2012/1/2"),
+ ... a.Instrument('lyra')) # doctest: +REMOTE_DATA

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

indentation error

@Cadair

Cadair May 3, 2018

Member

indentation error

+resulting search result determines the number of entries that will be added
+to the database. The number of entries that will be added depends on the total
+number of FITS headers. The :meth:`Database.download_from_fido_search_result()`
+method also accepts an optional keyword argument `path` which determines the

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

should be two backticks as it's not a code reference

@Cadair

Cadair May 3, 2018

Member

should be two backticks as it's not a code reference

+
+ >>> database.download_from_fido_search_result(search_result) # doctest: +REMOTE_DATA
+ >>> print(display_entries(database, ['id', 'observation_time_start',
+ ... 'observation_time_end', 'instrument', 'source'])) # doctest: +REMOTE_DATA

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

indentation

@Cadair

Cadair May 3, 2018

Member

indentation

+ for sr_entry, temp in zip(search_result, entries_list):
+ for database_entry in self:
+ if database_entry.path is not None and sr_entry._compare_attributes(database_entry,
+ ["source", "provider", "physobs", "fileid", "observation_time_start",

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

I would indent this a but more so it dosen't align to the following lines.

@Cadair

Cadair May 3, 2018

Member

I would indent this a but more so it dosen't align to the following lines.

+ delete_entries.append(database_entry)
+
+ for temp in remove_list:
+ search_result = [x for x in search_result if x != temp]

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

You could re-write this using sets rather than the for loop.

@Cadair

Cadair May 3, 2018

Member

You could re-write this using sets rather than the for loop.

This comment has been minimized.

@Punyaslok

Punyaslok May 3, 2018

Member

I remembered the reason not to use set difference for this. It will not maintain the ordering of the elements. Should I leave it as it is ?

@Punyaslok

Punyaslok May 3, 2018

Member

I remembered the reason not to use set difference for this. It will not maintain the ordering of the elements. Should I leave it as it is ?

+ for (path, sr_entry) in zip(paths, entries_list):
+
+ try:
+ read_file_header(path)

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

As discussed this probably shouldn't be here, as entries_from_file should raise if there is an error.

@Cadair

Cadair May 3, 2018

Member

As discussed this probably shouldn't be here, as entries_from_file should raise if there is an error.

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

My commit means you don't need this line anymore, I think

@Cadair

Cadair May 3, 2018

Member

My commit means you don't need this line anymore, I think

+ if os.path.isfile(path):
+ entries = tables.entries_from_file(path, self.default_waveunit)
+ elif os.path.isdir(path):
+ entries = tables.entries_from_dir(path, self.default_waveunit)

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

We should modify entries_from_dir to support returning a bare bones entry for non-sunpy io supported files.

@Cadair

Cadair May 3, 2018

Member

We should modify entries_from_dir to support returning a bare bones entry for non-sunpy io supported files.

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

Ok, so path should never actually be a directory, so we can probably just drop this line or replace it with an exception.

@Cadair

Cadair May 3, 2018

Member

Ok, so path should never actually be a directory, so we can probably just drop this line or replace it with an exception.

Punyaslok and others added some commits May 3, 2018

@@ -147,6 +147,8 @@ def download(self, url, path=None, callback=None, errback=None):
server = self._get_server(url)
# Create function to compute the filepath to download to if not set
+ #if path is not None:

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

are these supposed to be here?

@Cadair

Cadair May 3, 2018

Member

are these supposed to be here?

+ try:
+ instrument_name = next(x for x in entry.fits_header_entries if x.key == 'TELESCOP').value
+ except Exception:
+ pass

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

this should be instrument_name = '' so that it's defined below.

@Cadair

Cadair May 3, 2018

Member

this should be instrument_name = '' so that it's defined below.

+ if 'goes' in instrument_name.lower():
+ entry.observation_time_end = datetime.strptime(value,
+ '%d/%m/%Y')
+ # else:

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

uncomment this

@Cadair

Cadair May 3, 2018

Member

uncomment this

+ if 'goes' in instrument_name.lower():
+ entry.observation_time_start = datetime.strptime(value,
+ '%d/%m/%Y')
+ # else:

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

and this

@Cadair

Cadair May 3, 2018

Member

and this

dobj.download(aurl, fname, ncall, error_callback)
+ # dobj.download(aurl, kwargs.get('path', None), ncall,

This comment has been minimized.

@Cadair

Cadair May 3, 2018

Member

delete?

@Cadair

Cadair May 3, 2018

Member

delete?

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