Skip to content

Implement Spitzer Heritage Archive query#102

Merged
keflavich merged 17 commits into
astropy:masterfrom
autocorr:spitzer
Jun 5, 2013
Merged

Implement Spitzer Heritage Archive query#102
keflavich merged 17 commits into
astropy:masterfrom
autocorr:spitzer

Conversation

@autocorr

@autocorr autocorr commented Jun 3, 2013

Copy link
Copy Markdown
Contributor

I noticed that #45 mentioned querying the SHA through the SIAP protocol that they have setup. I've implemented the query to return a astropy.table.Table instance with the right dtypes using the requests library.

I'm not sure if this is redundant with the IRSA sub-module, but the SHA is mostly for querying meta-data and download links for image files, so it seems like it could be of use.

@autocorr

autocorr commented Jun 3, 2013

Copy link
Copy Markdown
Contributor Author

Oops, Looks like I added the lamda sub-module files to the branch I was working on. Only look at the files in astroquery/sha

@keflavich

Copy link
Copy Markdown
Contributor

Nice. I'll test this out before merging, but I think LAMDA is good to go so hopefully that will make the diff a little cleaner; otherwise I'll ask you to rebase.

A development note: when you make a new feature, you should make a new "feature branch" from upstream master. i.e., you would do:

git checkout master
git checkout -b lamda
git checkout master
git checkout -b spitzer_heritage

to make two different branches, both "branched from" master (there may be other ways to accomplish the same thing).

Also, on SHA specifically, there needs to be a way to get access to the image files as well. Presumably that's part of the metadata?

@keflavich

Copy link
Copy Markdown
Contributor

To get rid of the LAMDA commits, you can do:

git rebase -i upstream/master

(assuming you have 'upstream' defined - if not, do git remote add upstream git@github.com:astropy/astroquery.git)

then remove any lines that are from LAMDA commits and not from SHA commits.

After a rebase, you'll need to git push -f autocorr spitzer because you're changing history (-f = force)

@autocorr

autocorr commented Jun 3, 2013

Copy link
Copy Markdown
Contributor Author

Thanks for the help! Git is still confusing for me.

I've cleaned up the history with rebase like you suggested and added a function sha.get_img to get images/tables given the URLs returned by sha.query.

I haven't rigorously tested the routine, but I compared this file URL:

>>> t = sha.query(pid=30080)
>>> url = t['accessUrl'][16]
>>> sha.get_img(url)

with a version downloaded from sha.get_img and following the link in my browser and everything looks the same in ds9. There may be a more transparent way to retrieve a non-text file, but writing a binary stream was the only solution I found with the requests module.

Comment thread astroquery/sha/core.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you could replace this block with:

iofile = io.BytesIO(response.content)
fitsfile = fits.open(iofile)

(just import io and import astropy.io.fits as fits)

see vizier/core.py for an example

@keflavich

Copy link
Copy Markdown
Contributor

Good work on the rebase; it looks clean now.

In terms of reading binary files, originally we were using astropy.utils.data like this:

U = opener.open(link)
with aud.get_readable_fileobj(U) as f:
    results = f.read()

but since we're trying to move to requests, I suggest using the other approach I pointed out. At some point, we should probably unify all of this to a single get_fits_file_from_url command in utils...

@autocorr

autocorr commented Jun 3, 2013

Copy link
Copy Markdown
Contributor Author

They do have the metadata available on the API help page. But it actually may be easier in the SHA's case because the queries return a full URL for each file along with all the image/table information.

The method I found takes the requests response:

with open(path, 'wb') as f:
    for block in response.iter_content(1024):
        f.write(block)

The only thing you will need to know is what file-extension to stick on the end of the path name, or else it's not clear that the system will know how to open it. (The 1024 is only to speed up the number of bytes written for each chunk).

@keflavich

Copy link
Copy Markdown
Contributor

Ah, alright, if it's a zip file we can't return the open file. But my first example was to show how to parse a FITS file into a FITS object, rather than write it to disk. We'll need to refactor a lot of the file-getting stuff in the long term, but for now this is fine.

One last important thing to do before I merge this: Can you add a document docs/astroquery/sha.rst with a description of the query tool, and add it to the index (docs/astroquery/index.rst)?

Good stuff, I'm looking forward to using this one....

@autocorr

autocorr commented Jun 3, 2013

Copy link
Copy Markdown
Contributor Author

I see what you mean with the opening the fits file like vizier/core.py. Do you think that the functionality would be better to return a pyfits object rather than writing it straight to disk and letting the user open them as pyfits objects?

@keflavich

Copy link
Copy Markdown
Contributor

In general, yes. But since we haven't really implemented that for .zip files in particular, writing to disk is an OK temporary option (UKIDSS does that...).

@autocorr

autocorr commented Jun 3, 2013

Copy link
Copy Markdown
Contributor Author

Sorry, I'm asynchronous with the comments, and yes, I can update those!

@keflavich

Copy link
Copy Markdown
Contributor

Are the .zip files just gzip'd FITS files, or are they zip'd folders / collections of files?

@autocorr

autocorr commented Jun 3, 2013

Copy link
Copy Markdown
Contributor Author

The latter, they're zip'd folders of multiple calibration images (most of the time it seems). There seem to be three things to get from the SHA links found in the result queries: tables of metadata stuff, science frames as fits, and zip'd folders of calibration images. It would be easy to put a parameter that if it's a fits file, return a pyfits dealy rather than writing to disk.

@keflavich

Copy link
Copy Markdown
Contributor

Yes... this actually raises an important point, though. For zip'd folders, we may want to return an object, but it will be important that it is cached, and probably saving to disk makes more sense because a .zip file in memory is kind of useless...

@autocorr

autocorr commented Jun 3, 2013

Copy link
Copy Markdown
Contributor Author

Gotcha, well for the moment, I think it may be most transparent to have two functions a save_file and a get_file where the second returns a table, fits object, or list of fits objects if it's a zip file.

@autocorr autocorr mentioned this pull request Jun 4, 2013
4 tasks
@keflavich

Copy link
Copy Markdown
Contributor

Could you rebase this against master? They must have diverged in a different PR.

@autocorr

autocorr commented Jun 4, 2013

Copy link
Copy Markdown
Contributor Author

I rebased and did a forced update, did that fix things?

@keflavich

Copy link
Copy Markdown
Contributor

Still says I can't merge. There appears to be a conflict in the index.rst file. Try:

# only need this first command if you haven't already done it
git remote add upstream git@github.com:astropy/astroquery.git  
git remote update
git checkout spitzer
git rebase -i upstream/master

If that's what you already did, let me know and we can try something else...

@autocorr

autocorr commented Jun 4, 2013

Copy link
Copy Markdown
Contributor Author

When rebasing I pick all commits above (not deleting any) but get an error:

error: could not apply 0b453e0... Add lamda documentation page

@keflavich

Copy link
Copy Markdown
Contributor

OK, when you hit that point, that probably means you've hit a conflict. This time, it's pretty easy to fix (almost certainly just one line), but at this point type git status and have a look at the results. It should say docs/astroquery/index.rst is a conflict. Open that up in an editor and you should see the

>>>>>>>>
==========
<<<<<<<<

stuff marking the conflicting area. There are specific tools for dealing with this - try git mergetool and see if you have one of them installed (opendiff is default for mac).

@autocorr

autocorr commented Jun 5, 2013

Copy link
Copy Markdown
Contributor Author

Sorry to be somewhat lost on this, gvimdiff is my default mergetool, and I can see that alfalfa.rst got merged to the same line as where I put sha.rst.. I'm trying apply the changes at the moment

@keflavich

Copy link
Copy Markdown
Contributor

No worries, this is by far the most confusing part of version control. As a vim user, you might be interested in this tool:
https://github.com/whiteinge/dotfiles/blob/master/bin/diffconflicts

to set it up in your .gitconfig, add:

[mergetool "diffconflicts"]
    cmd = diffconflicts gvim $BASE $LOCAL $REMOTE $MERGED
    trustExitCode = true
    keepBackup = false

Brian Svoboda added 7 commits June 4, 2013 17:12
Changed from dictionary dependence to parameters in requests get. Using
the standard libs struct parser to parse the fixed width lines.
The query function works with appropriate data types and tested for
PID=30080 as on the API help page.
Brian Svoboda added 10 commits June 4, 2013 17:12
Added a method to get images and spectra from the URLs supplied by the
SHA query function. Issues could occur if there are different character
encodings or file types, but utf-8 and fits have covered all the tested
cases so far.
Commented out the actual get_img calls because they would create a
directoy and download files if run, potentially in parts of the file
system where the user/process does not have the right permissions.
@autocorr

autocorr commented Jun 5, 2013

Copy link
Copy Markdown
Contributor Author

Ok! I think that should have solved it, probably took a bit more googling that it should have.

@keflavich

Copy link
Copy Markdown
Contributor

OK, looks good, thanks!

keflavich added a commit that referenced this pull request Jun 5, 2013
Implement Spitzer Heritage Archive query
@keflavich keflavich merged commit 7406ddc into astropy:master Jun 5, 2013
@autocorr

autocorr commented Jun 5, 2013

Copy link
Copy Markdown
Contributor Author

Great, thanks for the help!

@autocorr autocorr deleted the spitzer branch June 5, 2013 00:31
keflavich pushed a commit to keflavich/astroquery that referenced this pull request Jan 12, 2015
keflavich pushed a commit to keflavich/astroquery that referenced this pull request Feb 5, 2015
updated docs Makefile to reflect astropy

Move the py.test conftest.py file to the astropy directory so it is installed. Changed the import of plugins accordingly. Also updated the .gitignore file so it ignores the Mac OS X .DS_Store file.

fixed conf.py to not insert source dir into sys.path

typo fixes in conf.py comment

Added CSS file from Astropy core package

Updated setup.cfg to match Astropy core package

Copy bootstrap-astropy theme from astropy

remove default.css import from astropy.css - now not needed

update conf.py to use bootstrap-astropy theme, mirror astropy's version

use installed astropy theme

Added templates that inherit from core package

Added `_templates` to list of patterns to exclude

added comment directing to base of jinja template

Update testing to only import pytest when tests are run

Added `coverage` option that was added in the Astropy core

Removed comments

added kwarg options to test runner

removed line setting unused _test_runner variable

Updated setup.py to bring it in line with the Astropy core

Update distribute_setup.py to 0.6.28

Remove unnecessary instructions from README.rst

tweak to instructions from @astrofrog's suggestion

Changed check in setup.py to syntax in core package to avoid issues when Sphinx is not installed.

Update setup.py to reflect re-factored setup code for 0.2 release

If necessary, move configuration file to the right place on first import

Added open_files option for test() and implemented fix for caught exception in __init__.py

Fix scripts/README.rst bug

added commented-out edit_on_github section to conf.py and related instructions in README

line wrap fixes

Add .travis.yml file

Add instructions for travis-ci to README

_PACKAGE_SETUP_ should be _ASTROPY_SETUP_ (see astropy/astropy#950)

Test with different Numpy versions, only test Sphinx with Python 2.7.

Ensure that optional dependencies for Sphinx are installed

Updated distribute_setup.py to fix compatibility with setuptools 0.7

README typo fixes

Switch to using ez_setup.py, and simplify setup.py, following recent changes in Astropy core.

Updates to __init__.py to match Astropy core

Minor fix

Added backward compatibility with Astropy 0.2

parallel option can only be used with Astropy 0.3 or later

Re-organize the setup.py so that Astropy is not required for egg_info

Revert "Re-organize the setup.py so that Astropy is not required for egg_info"

This reverts commit 9aa5339.

Replace distribute_setup.py with ez_setup.py

and ``setuptools_bootstrap.py``

Support `setup.py test --coverage` in affiliated packages.

Simplify package-template usage and updating by requiring fewer files to
be edited.

Add information about pulling changes to package template

Store stuff in "metadata" section, not "affiliated" section.

Insert space before comment.

Fix documentation build

Elaborate on the need to search for .c files in a comment.

Change ``clean`` rule to remove generated docs

Made necessary by astropy/astropy@b94fd38

Update to newer ez_setup.py

Add the current version of ah_bootstrap--for the tests themselves we will always copy over the latest version of ah_bootstrap (so that we can test ah_bootstrap itself), but there should still be a default copy of it in the package-template repo as well

Configure the package-template to use astropy_helpers.  By default includes astropy_helpers as a submodule. Yes, astropy_helpers is now a submodule of package_template which is a submodule of astropy_helpers

update the astropy_helpers version to include fix from c3f841b9e099f506a751b8a6971b15e7e3d6590a

Update the astropy_helpers version used in the package_template

Update the astropy_helpers version again; use the auto_use feature of ah_bootstrap.

Switch to using astropy_helpers for docs build.

Updated astropy_helpers version (included fix for Python 3)

Update the git submodule path to use http instead--hopefully this will work then on Travis

Updating to the actual astropy-helpers

Updated astropy-helpers to latest version

Fixing docs links in package test function

Added example API docs

Updated astropy-helpers to latest version.

This version includes fixes for the coverage testing in affiliated packages.

Use wheels for travis testing

Care is taken to use a numpy wheel whose version matches the scipy/astropy wheels.

Previous commit messages:
Use development astropy for test --coverage
Install coverage when needed
Add cython to the travis build environment, controlled by an environmental variable
Also added a few more words of advice to the travis part of the README
Restore -w option to build_sphinx
Change URL for numpy-specific wheels and make scipy an optional install
Fix latest stable numpy version
Several tweaks to improve performance and reduce unnecessary tests
Remove debugging statements and cython install in egg_only

Don't override package data after it's been determined from
setup_package.py files

Include astropy_helpers in the affilated package's tarball.

Fix astropy#69: Affiliated packages can opt-in to deprecations-as-exceptions

removed setuptools_bootstrap.py as it doesn't exist any more in the new package-template

added ah_bootstrap and setup.cfg

Update astropy_helpers

Use MiniConda for affiliated packages

Make sure conda’s astropy is removed before pip installing the latest developer version

Added more comments to .travis.yml

Pin Numpy version

Don’t use --parallel with Python 3

More tidying up of .travis.yml file

Move astropy dev builds earlier in .travis.yml

Since they take longer, it is better to start them sooner for improved load balancing.

Import astropy_helpers from inside the docs/ directory

Fix a warning that "Could not determine version of package ccdproc" from
configuration system (since the __version__ member doesn't yet exist for
the package when the configuration file is copied).

More detail in example __init__.py

Add numpy import to exmaple_mod

This ensures that egg_info builds will break if __init__.py is not set up properly

Add coveralls --rcfile option

Update README for coveralls rcfile

Use the version module correctly in edit_on_github

Closes astropy#55.

Added files to ignore in affiliated package coverage tests

Clarify purpose of cextern directory.

Updated astropy-helpers to v0.4

Update ah_bootstrap.py from astropy-helpers

Replace development build with stable now that astropy 0.4 is out

Closes astropy#65

Add info about a specific Sphinx version check.

explain readthedocs

add released versions of astropy-helper and astropy to rtd-requirements

Add a few entries to .gitignore

Remove affiliated package instructions from README page

update readme with a bit more info

Typo fix pointed out by @cdeil

Updated astropy-helpers and ah_bootstrap.py to v0.4.1

Added TEMPLATE_CHANGES.md. Fixes astropy#102.

MANIFEST.in should exclude pyc *after* including astropy-helpers

prune docs/api

gitignore manifest

template changes udpate

Updated astropy-helpers to v0.4.3

Update TEMPLATE_CHANGES.md

Fix typo

Tagging v0.4.1

Updated astropy-helpers to patched v0.4.3

Update dependency information for open-files option

Add note about adding/removing packages to/from pytest header

Ensure that conftest.py change works with all versions of Astropy

Make sure to update apt-get listings

Add jinja2 in .travis.yml

Drop python 3.2 testing

Drop numpy 1.5 testing and use numpy 1.9 as baseline

Use setuptools entry_points for command line scripts

Update MANIFEST.in [skip ci]

Update TEMPLATE_CHANGES [skip ci]

add powered by astropy badge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants