Use rustfits or astropy remote file access when possible. - #178
Conversation
erykoff
changed the title
|
|
||
| self.fits_object = rustfits.FITS(filename, mode=rustfits_mode) | ||
| if mode == "r" and protocol != "file": | ||
| self.fits_object = rustfits.FITS(filename, mode=rustfits_mode, remote="ranged") |
There was a problem hiding this comment.
Is this defaulting to always using ranged? I think often this is not the best choice, much faster to use the default "download all and work with it in memory"
There was a problem hiding this comment.
Hmm. Part of the problem is that only rustfits supports this. The other thing is that I was thinking that if somebody is handing a remote URI then they are asking to read it remotely (for good or ill). I also worry about trying to get too many parameters in at the top and passing them all down ... but if you think it's necessary I could.
There was a problem hiding this comment.
what is only supported by rustfits?
There was a problem hiding this comment.
Right now I support various fits backends, but I am thinking that I should remove that ... or at least remove fitsio because I do require rustfits. So I'll do that cleanup on this PR.
But I also support astropy fits reading which is currently required for remote access via S3 or google cloud or webdav (I don't think rustfits supports these or it would be a great surprise). And only rustfits will "download the file to a temp cache and then read it". I personally find that behavior surprising. I would instead expect that if I give a remote protocol I want to read it remotely.
There was a problem hiding this comment.
Getting this to work took some real code and decisions, where as the download version didn't take any.
Also the download is what cfitsio does.
From that point of view, ranged access not being default isn't a surprise, the ranged read existing at all is the nice addition (thanks for prompting it). It is nice astropy does it too.
Why does S3, google cloud, webdave require anything other than authentication? rustfits does support that through the keyword
There was a problem hiding this comment.
| fitsio>=1.4.0 | ||
| rustfits = | ||
| rustfits>=0.1.5 | ||
| rustfits>=0.1.7 |
There was a problem hiding this comment.
I think ranged landed in 0.1.8
2 participants
No description provided.