Support for fsspec filesystem configuration via fits.open by bmorris3 · Pull Request #19294 · astropy/astropy

bmorris3 · 2026-02-12T01:17:56Z

Description

This pull request is a proposal to address #19292.

Remote FITS files can be partially read from S3 or Google Cloud through fits.open via fsspec.open, which identifies the remote file system and opens the URI using an instance of fsspec.filesystem with the default parameters. fsspec.open works in many cases, but does not give finer control over, e.g., the (buffer) block size.

The remote FITS docs contain an example for configuring block size:

fsspec_kwargs = {"block_size": 1_000_000, "cache_type": "bytes"}
with fits.open(url, use_fsspec=True, fsspec_kwargs=fsspec_kwargs) as hdul:
    cutout = hdul[1].section[10:20, 30:50]

but the argument to fits.open is url rather than s3_uri, which perhaps was intended. This is why doctests have been passing, though running the above snippet will fail with an S3 prefix in place of url.

UPDATE: the first API proposal has evolved. The old API suggestion is expandable below:

old API proposal

The proposed fix in this PR adds a keyword argument to get_readable_fileobj called filesystem, which get passed into fsspec.filesystem. fsspec.filesystem is the constructor which takes block_size and cache_type, and also anon.

The revised example looks like this:

## from earlier in docs: s3_uri = "s3://stpubdata/hst/public/j8pu/j8pu0y010/j8pu0y010_drc.fits"

filesystem_kwargs = {"anon": True, "block_size": 1_000_000, "cache_type": "bytes"}
with fits.open(s3_uri, filesystem_kwargs=filesystem_kwargs) as hdul:
    cutout = hdul[1].section[10:20, 30:50]

To maintain backwards compatibility, I've made double-use of fsspec_kwargs. If filesystem_kwargs is None (default), fsspec_kwargs gets passed to fsspec.open, as it always has. If filesystem_kwargs is not None, fsspec_kwargs gets passed to fsspec.filesystem.open. This way a user can control both the file system configuration and the file open configuration, if they want. I understand if folks think that's tricky, and I'm open to suggestions.

The proposed fix in this PR adds a keyword argument to get_readable_fileobj called fsspec_filesystem, which takes an fsspec.filesystem. This constructor takes configurable arguments like protocol, block_size and cache_type, and also anon.

The revised example looks like this:

## from earlier in docs: s3_uri = "s3://stpubdata/hst/public/j8pu/j8pu0y010/j8pu0y010_drc.fits"

import fsspec

filesystem = dict(block_size=1_000_000, cache_type="bytes")
kwargs = dict(anon=True)
with fits.open(s3_uri, fsspec_filesystem=filesystem, fsspec_kwargs=kwargs) as hdul:
    cutout = hdul[1].section[10:20, 30:50]

As before, fsspec_kwargs gets passed to fsspec.open, as it always has.

Fixes #19292

By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

github-actions · 2026-02-12T01:18:08Z

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

Do the proposed changes actually accomplish desired goals?
Do the proposed changes follow the Astropy coding guidelines?
Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

bmorris3 · 2026-02-12T13:45:57Z

Thanks @pllim! Is it ok to add fsspec to the intersphinx mapping? It certainly would have helped me when I was flipping between docs.

bmorris3 · 2026-02-12T14:00:24Z

Ignore the above, fsspec is already in the mapping.

bsipocz

The code changes look good, thanks!

However, these 2 items needs to be addressed before merging:

The new kwarg needs test coverage,
the narrative docs needs an update,

bsipocz · 2026-02-12T03:35:45Z


        .. versionadded:: 5.2

+    filesystem_kwargs : dict, optional


what about making this even more explicit, e.g. fsspec_filesystem_kwargs?

I'm ok with that, only opted to drop fsspec_ because all together it's a lot of characters.

I think the clarity is worth the extra characters; and if abbreviation is needed then I would instead drop filesystem for fs; but still think the whole spell out is worth it for this case.

bsipocz · 2026-02-12T03:38:27Z

 Configuring the ``fsspec`` block size and download strategy
 -----------------------------------------------------------

 The ``fsspec`` package supports different data reading and caching strategies
 which aim to find a balance between the number of network requests on one hand
 and the total amount of data transferred on the other hand.  By default, fsspec
 will attempt to download data in large contiguous blocks using a buffered
 *read ahead* strategy, similar to the strategy that is employed when operating
 systems load local files into memory.

 You can tune the performance of fsspec's buffering strategy by passing custom
 ``block_size`` and ``cache_type`` parameters to `fsspec.open`.  You can pass
 these parameters via the ``fsspec_kwargs`` argument of `astropy.io.fits.open`.
 For example, we can configure fsspec to make buffered reads with a minimum
 ``block_size`` of 1 MB as follows:


Please update this documentation

timj · 2026-02-12T17:05:10Z

What I've been doing is creating the fsspec object myself and then doing:

with fs.open(fspath) as f, astropy.io.fits.open(f) as fits_obj:
    ...

which works great (but might not be documented to work) and is in some sense nicely explicit, but allowing fsspec parameters to be passed in directly to open makes sense so long as we are saying fsspec is the internal interface forever.

bsipocz

Thanks for addressing the review, this LGTM now.

pllim · 2026-02-12T21:55:46Z


        .. versionadded:: 5.2

+    fsspec_filesystem_kwargs : dict, optional


The order should match signature above.

yikes, good catch!

(the docstring was right but the call signature wasn't.)

pllim · 2026-02-12T21:56:10Z

@@ -0,0 +1,4 @@
+Fix for ``fsspec`` configuration in ``fits.open`` for remote S3 objects. ``fits.open``
+now accepts the keyword argument ``filesystem_kwargs``, which is passed onto


This needs updating.

pllim · 2026-02-12T21:57:12Z

@bsipocz , why remove the API change label?

timj · 2026-02-12T21:57:58Z

@@ -0,0 +1,4 @@
+Fix for ``fsspec`` configuration in ``fits.open`` for remote S3 objects. ``fits.open``


Is this really only of use for S3 URIs? I had assumed it was generically useful for any supported fsspec URI scheme that could take additional parameters.

Good catch, I've exposed my bias here.

bsipocz · 2026-02-12T22:08:19Z

@bsipocz , why remove the API change label?

As it adds to the API rather than changing it

pllim · 2026-02-12T22:17:17Z

Hmm I see this in pre-deps. Is it transient or has to be fixed?

utils/tests/test_data.py::test_get_readable_fileobj_fsspec[kwargs1] - botocore.exceptions.NoCredentialsError: Unable to locate credentials
FAILED docs/io/fits/usage/cloud.rst::cloud.rst

pllim · 2026-02-12T22:18:20Z

It would have showed up in devdeps too that run remote data but that job is currently messed up by #19216

bmorris3 · 2026-02-13T14:43:45Z

Hmm I see this in pre-deps. Is it transient or has to be fixed?

utils/tests/test_data.py::test_get_readable_fileobj_fsspec[kwargs1] - botocore.exceptions.NoCredentialsError: Unable to locate credentials
FAILED docs/io/fits/usage/cloud.rst::cloud.rst

This one is on me, fixing now.

saimn · 2026-02-16T18:10:05Z

Having 2 parameters, one of which meaning something different depending on the value of the first one, seems a bit complicated to me... If there is an alternative with passing a fsspec filesystem object as in @timj's example (#19294 (comment)), I would favor this option, it seems more explicit and flexible.

bmorris3 · 2026-02-17T15:23:44Z

@saimn I agree that the API is awkward. I also think the point of the cloud FITS feature via fits.open is to handle these things for the user. I'm not opposed to passing in an initialized filesystem object. I'll make that change now.

neutrinoceros

got a superficial nit left, but otherwise I'll lift my request for changes as all my important remarks were addressed.

neutrinoceros · 2026-03-10T17:15:43Z

closing/re-opening to check against newly added CI job since this PR adds (optional) dependencies

(ref #19022)

neutrinoceros · 2026-03-17T16:19:26Z

@bmorris3 please pay attention to https://github.com/astropy/astropy/actions/runs/23204311204/job/67435317522?pr=19294

While not (yet) required because it's brand new, this job failing indicates that action needs to be taken. It should be clear what to do from the logs, but please let me know if not. Thanks !

…esystem directly

Co-authored-by: Brigitta Sipőcz <[email protected]>

Co-authored-by: Clément Robert <[email protected]>

bmorris3 · 2026-03-23T18:46:42Z

@bsipocz May I ask for a rereview? Thanks

bsipocz

This still looks good to me (I mean my blocking review was only due to the failing tests rather than the code itself. Tests are passing, so this should be good to go).

pllim · 2026-03-23T20:26:09Z

Merged. Thanks, all!

bmorris3 requested a review from saimn as a code owner February 12, 2026 01:17

github-actions Bot added Docs io.fits utils labels Feb 12, 2026

pllim added this to the v8.0.0 milestone Feb 12, 2026

pllim added Bug API change PRs and issues that change an existing API, possibly requiring a deprecation period labels Feb 12, 2026

pllim requested review from barentsen and bsipocz February 12, 2026 01:47

This comment was marked as resolved.

Sign in to view

bmorris3 force-pushed the bugfix-fsspec-kwargs branch from 14d6d06 to 758b872 Compare February 12, 2026 14:07

bsipocz requested changes Feb 12, 2026

View reviewed changes

bsipocz removed the API change PRs and issues that change an existing API, possibly requiring a deprecation period label Feb 12, 2026

bsipocz approved these changes Feb 12, 2026

View reviewed changes

pllim added the Extra CI Run cron CI as part of PR label Feb 12, 2026

pllim reviewed Feb 12, 2026

View reviewed changes

timj reviewed Feb 12, 2026

View reviewed changes

pllim mentioned this pull request Feb 12, 2026

BUG: numpy.char.chararray is deprecated #19216

Closed

bmorris3 requested review from bsipocz and neutrinoceros March 10, 2026 15:49

neutrinoceros reviewed Mar 10, 2026

View reviewed changes

Comment thread docs/io/fits/usage/cloud.rst Resolved

neutrinoceros approved these changes Mar 10, 2026

View reviewed changes

neutrinoceros closed this Mar 10, 2026

neutrinoceros reopened this Mar 10, 2026

bmorris3 force-pushed the bugfix-fsspec-kwargs branch from 5a1e21c to cb5bda0 Compare March 17, 2026 16:15

bmorris3 mentioned this pull request Mar 19, 2026

bugfix: stream ASDF from S3 spacetelescope/jdaviz#3980

Merged

10 tasks

bmorris3 requested a review from pllim March 20, 2026 15:04

bmorris3 and others added 14 commits March 20, 2026 11:05

support for fsspec filesystem configuration via fits.open

65fb0a8

add changelog, fix intersphinx mapping

9be48ff

adding test coverage

787e35e

addressing review comments

ee3304c

switching from passing fsspec.filesystem kwargs to passing fsspec.fil…

1242a10

…esystem directly

docs fix

09adfec

fix test to use test_all deps

c12f7b4

Update pyproject.toml

0161f03

Co-authored-by: Brigitta Sipőcz <[email protected]>

optional dep is fsspec (not just http)

d8c4727

pull in s3fs dep through fsspec[full]

7afadb1

fix anonymous access

7a21389

minimize fsspec optional deps

5872aa2

Update docs/io/fits/usage/cloud.rst

6f06d94

Co-authored-by: Clément Robert <[email protected]>

update solution to dependency constraints

fd8b2ae

bmorris3 force-pushed the bugfix-fsspec-kwargs branch from 2213937 to fd8b2ae Compare March 20, 2026 15:05

bsipocz approved these changes Mar 23, 2026

View reviewed changes

pllim merged commit 4e43524 into astropy:main Mar 23, 2026
40 of 42 checks passed


		.. versionadded:: 5.2

		fsspec_filesystem_kwargs : dict, optional

		@@ -0,0 +1,4 @@
		Fix for ``fsspec`` configuration in ``fits.open`` for remote S3 objects. ``fits.open``
		now accepts the keyword argument ``filesystem_kwargs``, which is passed onto

Conversation

bmorris3 commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

github-actions Bot commented Feb 12, 2026

Uh oh!

This comment was marked as resolved.

bmorris3 commented Feb 12, 2026

Uh oh!

bmorris3 commented Feb 12, 2026

Uh oh!

bsipocz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timj commented Feb 12, 2026

Uh oh!

bsipocz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pllim commented Feb 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bsipocz commented Feb 12, 2026

Uh oh!

pllim commented Feb 12, 2026

Uh oh!

pllim commented Feb 12, 2026

Uh oh!

bmorris3 commented Feb 13, 2026

Uh oh!

saimn commented Feb 16, 2026

Uh oh!

bmorris3 commented Feb 17, 2026

Uh oh!

Uh oh!

neutrinoceros left a comment

Choose a reason for hiding this comment

Uh oh!

neutrinoceros commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neutrinoceros commented Mar 17, 2026

Uh oh!

bmorris3 commented Mar 23, 2026

Uh oh!

bsipocz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pllim commented Mar 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

bmorris3 commented Feb 12, 2026 •

edited

Loading

neutrinoceros commented Mar 10, 2026 •

edited

Loading

bsipocz left a comment •

edited

Loading