Skip to content

Add a generic load_file method#78

Closed
pllim wants to merge 3 commits intoastropy:mainfrom
pllim:load_file_meth
Closed

Add a generic load_file method#78
pllim wants to merge 3 commits intoastropy:mainfrom
pllim:load_file_meth

Conversation

@pllim
Copy link
Member

@pllim pllim commented Mar 21, 2019

This would fix #64 indirectly, as theoretically I could implement whatever Ginga file handler I want outside of the widget, register it with Ginga, and then simply use this to open my custom file format.

https://github.com/ejeschke/ginga/blob/bbfe6455d302266c1bfd3f01bd386fbc2494f3db/ginga/AstroImage.py#L186

I can add a test if you think this is an acceptable way forward. I am open to discussions. Thanks!

Depends on: ejeschke/ginga#764

cc @ejeschke

@pllim pllim added the enhancement New feature or request label Mar 21, 2019
@pllim pllim requested review from eteq and mwcraig March 21, 2019 20:24
Copy link
Member

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Maybe add a test?

@pllim
Copy link
Member Author

pllim commented Mar 25, 2019

Thanks for the review, @mwcraig ! I added a test, as requested.

@pllim
Copy link
Member Author

pllim commented Mar 25, 2019

p.s. Do I also have to open an issue in @eteq 's API thingy?

kwargs : dict
Extra keywords to be passed into the viewer's file handler.

"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pllim, you might want to use:

from ginga.util import loader
image = loader.load_data(filename, **kwargs)
if not isinstance(image, AstroImage):
    raise ValueError("Default index in '{}' does not reference an image".format(filename))
self._viewer.set_image(image)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a parameter for specifying HDU or some kind of index in the astrowidget load API? Because you could pass it in to idx in this load_data call.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new method is meant more for ASDF than for FITS, though ideally it would work on either one if the Ginga file loader registry is set up properly.

@pllim
Copy link
Member Author

pllim commented Mar 29, 2019

@ejeschke , test is failing because ginga.util.loader is not in the stable version yet. I guess we'll have to wait for a Ginga release? p.s. There is no rush.

@mwcraig
Copy link
Member

mwcraig commented Apr 7, 2019

p.s. Do I also have to open an issue in @eteq 's API thingy?

May as well, yes

Base automatically changed from master to main March 9, 2021 22:47
@pllim pllim closed this Jun 16, 2023
@pllim pllim deleted the load_file_meth branch June 16, 2023 15:10
@pllim
Copy link
Member Author

pllim commented Jun 16, 2023

We agreed to do refactor this file loading API and have it be backend agnostic. See notes from Astropy Coordination Meeting 2023.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ENH: Add interface to Ginga's ASDF loader

3 participants