Adds support for file-handlers and file-type in Map and TimeSeries #5193

devanshshukla99 · 2021-04-07T09:51:12Z

Description

Fixes #2690

Adds support for file-object and file-type in sunpy.map.Map & sunpy.timeseries.TimeSeries.

Currently, the support for sunpy.timeseries.TimeSeries is broken

pep8speaks · 2021-04-07T09:51:19Z

Hello @devanshshukla99! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers!

Comment last updated at 2021-07-24 08:00:42 UTC

changelog/5193.doc.rst

changelog/5193.trivial.rst

changelog/5193.feature.rst

sunpy/io/ana.py

sunpy/map/map_factory.py

sunpy/timeseries/tests/test_timeseries_factory.py

sunpy/timeseries/timeseries_factory.py

devanshshukla99 · 2021-04-09T10:31:47Z

@nabobalis I can't find why test py37-figure-devdeps is failing even though tox is reporting no bug, could you please help me regarding this?

nabobalis · 2021-04-09T10:37:55Z

The hash has changed. You can ignore it.

sunpy/io/ana.py

sunpy/map/map_factory.py

sunpy/io/ana.py

sunpy/map/mapbase.py

sunpy/io/file_tools.py

devanshshukla99 · 2021-05-11T22:17:45Z

Sure add a test for detect_filetype.

@nabobalis is there any hdf5 test file in sunpy?

nabobalis · 2021-05-11T22:27:26Z

Sure add a test for detect_filetype.

@nabobalis is there any hdf5 test file in sunpy?

https://github.com/sunpy/sunpy/blob/main/sunpy/data/test/goes_truncated_test_goes15.nc
and
https://github.com/sunpy/sunpy/blob/main/sunpy/data/test/goes_truncated_test_goes16.nc
should work

sunpy/timeseries/timeseries_factory.py

nabobalis · 2021-05-19T17:02:34Z

So just to double check, we want any places where the code cannot use an open file handler to error instead of use the filename and then open the file.

devanshshukla99 · 2021-05-20T16:27:51Z

As discussed during the dev meeting, it'd be good to support (and test!) file-like objects that do not actually correspond to a real on-disk file. For example, socket.socket.makefile() returns a file-like object for a socket.

So, if I understand correctly, we want to support socket file-obj in sunpy.io.read_file for reading remote files?

If so, don't we'll still have to download the file locally for processing considering socket file-obj aren't seekable?

nabobalis · 2021-07-24T08:28:07Z

sunpy/map/tests/test_map_factory.py

@@ -98,6 +102,11 @@ def test_patterns(self):
        assert isinstance(maps, list)
        assert ([isinstance(amap, sunpy.map.GenericMap) for amap in maps])

+        # File-like object
+        with open(a_fname, "rb") as fd:


What happens if you pass random binary data to map?

it returns a ValueError

Is there a test for this in this file?

Not currently, should I add one?

If you could.

nabobalis · 2021-07-24T08:29:20Z

sunpy/io/file_tools.py

@@ -59,7 +61,7 @@ def read_file(filepath, filetype=None, **kwargs):

    Parameters
    ----------
-    filepath : path-like
+    filepath : `str` or file-like


Does a pathlib.Path not work?

no it works,
will update👍

nabobalis · 2021-07-24T09:37:34Z

As discussed during the dev meeting, it'd be good to support (and test!) file-like objects that do not actually correspond to a real on-disk file. For example, socket.socket.makefile() returns a file-like object for a socket.

Any work on this? Or we could open an issue to support this in future?

devanshshukla99 · 2021-07-24T10:06:16Z

As discussed during the dev meeting, it'd be good to support (and test!) file-like objects that do not actually correspond to a real on-disk file. For example, socket.socket.makefile() returns a file-like object for a socket.

Any work on this? Or we could open an issue to support this in future?

Nothing till now.

So, if I understand correctly, we'll have to read the socket fileobj then pass the data to appropriate readers?
(considering the file-object returned by socket is not seekable)

nabobalis · 2021-07-24T10:10:47Z

So, if I understand correctly, we'll have to read the socket fileobj then pass the data to appropriate readers?
(considering the file-object returned by socket is not seekable)

Sure, his use case was makefile which returns a file-like handler.

devanshshukla99 · 2021-07-24T10:42:46Z

So, if I understand correctly, we'll have to read the socket fileobj then pass the data to appropriate readers?
(considering the file-object returned by socket is not seekable)

Sure, his use case was makefile which returns a file-like handler.

Looks doable!
Will do👍

nabobalis · 2021-09-15T16:38:44Z

I think we can bump the socket request from Albert to an issue and leave this PR as it for review.

@devanshshukla99, could you rebase this PR for us?

nabobalis · 2021-10-27T16:43:07Z

Hi @devanshshukla99, sorry for the lack of reviews on this PR.

I am trying to get someone to have a look but could you also rebase this if you have time? If not, I can do so.

devanshshukla99 · 2021-11-03T15:21:52Z

Hi @devanshshukla99, sorry for the lack of reviews on this PR.

I am trying to get someone to have a look but could you also rebase this if you have time? If not, I can do so.

Apologies from my side too
Will do 👍🏻

Cadair

Thanks for the work on this @devanshshukla99, sorry it took so long for me to review it. I think it's close, I just think we need to remove the last bit of name checking stuff for file handlers.

Cadair · 2021-11-03T16:39:19Z

sunpy/io/ana.py

@@ -44,6 +45,9 @@ def read(filename, debug=False, **kwargs):
    --------
    >>> data = sunpy.io.ana.read(filename)  # doctest: +SKIP
    """
+    if isinstance(filename, io.IOBase):
+        raise TypeError("Reader does not support file-handler")


Suggested change

raise TypeError("Reader does not support file-handler")

raise TypeError("The ANA reader does not support file handles.")

Cadair · 2021-11-03T16:41:22Z

sunpy/io/jp2.py

@@ -26,6 +27,9 @@ def read(filepath, **kwargs):
    pairs : `list`
        A list of (data, header) tuples.
    """
+    if isinstance(filepath, io.IOBase):
+        raise TypeError("Reader does not support file-handler")


Suggested change

raise TypeError("Reader does not support file-handler")

raise TypeError("The ANA reader does not support file handles.")

Cadair · 2021-11-03T16:47:40Z

sunpy/io/file_tools.py

@@ -77,19 +79,24 @@ def read_file(filepath, filetype=None, **kwargs):
    -----
    Other keyword arguments are passed to the reader used.
    """
-    filepath = str(pathlib.Path(filepath))
+    _filepath = filepath.name if isinstance(filepath, io.IOBase) else filepath


Not all objects which implement io.IOBase have a .name attribute:

>>> import io >>> isinstance(io.BytesIO(), io.IOBase) True >>> io.BytesIO().name AttributeError: '_io.BytesIO' object has no attribute 'name'

I don't think we should do this at all when dealing with io objects, I think it would be better to work out the type from the magic bytes at the start of the file and not any reference to the filename. (Which already happens if this filename doesn't match anything I think).

Cadair · 2021-11-03T16:49:41Z

sunpy/io/tests/test_filetools.py

@@ -44,6 +44,15 @@ def test_read_file_fits(self):
        assert all([isinstance(p[1],
                               sunpy.io.header.FileHeader) for p in pairs])

+        # Test file-object


We should add a test here to check it works as expected with other file objects, something like this:

with open(AIA_171_IMAGE, 'rb') as fd: fits_bytes = fd.read() sunpy.io.read_file(io.BytesIO(fits_bytes)) ...

Cadair · 2021-11-03T16:51:57Z

sunpy/map/tests/test_map_factory.py

+        eitmap = sunpy.map.Map(a_fname, filetype=pathlib.Path(a_fname).suffix[1:])
+        assert isinstance(eitmap, sunpy.map.GenericMap)


You aren't really testing this properly here as you are giving it a filename which it can detect the type for. You would be better off giving it a random filename and the type.

(Although it will also use magic bytes to detect file type if the filename doesn't pan out so I am not sure exactly how to make this test fail.)

nabobalis added map Affects the map submodule No Backport A PR that isn't to be backported to any release branch. (To be used as a flag to other maintainers) timeseries Affects the timeseries submodule labels Apr 7, 2021

devanshshukla99 marked this pull request as ready for review April 8, 2021 07:13

devanshshukla99 requested review from a team as code owners April 8, 2021 07:13

nabobalis removed request for a team April 8, 2021 10:28

nabobalis reviewed Apr 8, 2021

View reviewed changes

devanshshukla99 marked this pull request as draft April 8, 2021 11:32

devanshshukla99 marked this pull request as ready for review April 10, 2021 11:04

Cadair self-requested a review April 14, 2021 16:57

nabobalis reviewed Apr 14, 2021

View reviewed changes

sunpy/io/ana.py Outdated Show resolved Hide resolved

nabobalis reviewed Apr 14, 2021

View reviewed changes

sunpy/map/map_factory.py Outdated Show resolved Hide resolved

nabobalis added the Needs Review Needs reviews before merge label Apr 20, 2021

nabobalis removed the request for review from Cadair May 10, 2021 19:50

nabobalis reviewed May 10, 2021

View reviewed changes

sunpy/io/ana.py Outdated Show resolved Hide resolved

nabobalis reviewed May 10, 2021

View reviewed changes

sunpy/map/mapbase.py Outdated Show resolved Hide resolved

nabobalis reviewed May 10, 2021

View reviewed changes

sunpy/io/file_tools.py Outdated Show resolved Hide resolved

devanshshukla99 force-pushed the fix-2690 branch 2 times, most recently from db7ee52 to 5782674 Compare May 11, 2021 21:44

nabobalis reviewed May 19, 2021

View reviewed changes

sunpy/timeseries/timeseries_factory.py Outdated Show resolved Hide resolved

devanshshukla99 force-pushed the fix-2690 branch 5 times, most recently from 8b762c8 to 2b27902 Compare June 1, 2021 20:48

Cadair self-requested a review July 7, 2021 16:46

devanshshukla99 and others added 6 commits July 24, 2021 12:51

Added support for file-objects and filetype

76b716f

Added Tests for fileobj in ana, jp2 and fits

9a50c12

ana and jp2 will now error out with file-handler.

4678884

added test for detect_filetype

fa27a82

fix codestyle

b906b5b

fix

ac82e7f

devanshshukla99 force-pushed the fix-2690 branch from 2b27902 to ac82e7f Compare July 24, 2021 08:00

nabobalis reviewed Jul 24, 2021

View reviewed changes

devanshshukla99 marked this pull request as draft July 24, 2021 09:05

nabobalis marked this pull request as ready for review August 25, 2021 16:23

Cadair requested changes Nov 3, 2021

View reviewed changes

dstansby removed the Needs Review Needs reviews before merge label Jan 19, 2022

nabobalis marked this pull request as draft January 24, 2022 21:38

dstansby added the Needs Adoption PRs that were abandoned but should be picked up again and merged in label Jan 25, 2022

nabobalis closed this May 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for file-handlers and file-type in Map and TimeSeries #5193

Adds support for file-handlers and file-type in Map and TimeSeries #5193

devanshshukla99 commented Apr 7, 2021 •

edited

pep8speaks commented Apr 7, 2021 •

edited

devanshshukla99 commented Apr 9, 2021

nabobalis commented Apr 9, 2021

devanshshukla99 commented May 11, 2021

nabobalis commented May 11, 2021

nabobalis commented May 19, 2021

devanshshukla99 commented May 20, 2021 •

edited

nabobalis Jul 24, 2021

devanshshukla99 Jul 24, 2021

nabobalis Jul 24, 2021

devanshshukla99 Jul 24, 2021

nabobalis Jul 24, 2021

nabobalis Jul 24, 2021

devanshshukla99 Jul 24, 2021

nabobalis commented Jul 24, 2021

devanshshukla99 commented Jul 24, 2021

nabobalis commented Jul 24, 2021

devanshshukla99 commented Jul 24, 2021

nabobalis commented Sep 15, 2021 •

edited

nabobalis commented Oct 27, 2021

devanshshukla99 commented Nov 3, 2021

Cadair left a comment

Cadair Nov 3, 2021

Cadair Nov 3, 2021

Cadair Nov 3, 2021

Cadair Nov 3, 2021

Cadair Nov 3, 2021

	raise TypeError("Reader does not support file-handler")
	raise TypeError("The ANA reader does not support file handles.")

		eitmap = sunpy.map.Map(a_fname, filetype=pathlib.Path(a_fname).suffix[1:])
		assert isinstance(eitmap, sunpy.map.GenericMap)

Adds support for file-handlers and file-type in Map and TimeSeries #5193

Adds support for file-handlers and file-type in Map and TimeSeries #5193

Conversation

devanshshukla99 commented Apr 7, 2021 • edited

Description

pep8speaks commented Apr 7, 2021 • edited

Comment last updated at 2021-07-24 08:00:42 UTC

devanshshukla99 commented Apr 9, 2021

nabobalis commented Apr 9, 2021

devanshshukla99 commented May 11, 2021

nabobalis commented May 11, 2021

nabobalis commented May 19, 2021

devanshshukla99 commented May 20, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabobalis commented Jul 24, 2021

devanshshukla99 commented Jul 24, 2021

nabobalis commented Jul 24, 2021

devanshshukla99 commented Jul 24, 2021

nabobalis commented Sep 15, 2021 • edited

nabobalis commented Oct 27, 2021

devanshshukla99 commented Nov 3, 2021

Cadair left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devanshshukla99 commented Apr 7, 2021 •

edited

pep8speaks commented Apr 7, 2021 •

edited

devanshshukla99 commented May 20, 2021 •

edited

nabobalis commented Sep 15, 2021 •

edited