Skip to content

Conversation

@cdeil
Copy link
Member

@cdeil cdeil commented Apr 22, 2015

@cdeil cdeil added this to the Wishlist milestone Apr 22, 2015
@abigailStev
Copy link
Contributor

Here's a link for the Batch Browse interface, with an example: https://heasarc.gsfc.nasa.gov/W3Browse/w3batchinfo.html

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 66.22% when pulling 83fed16 on cdeil:heasarc into 2bbb5c4 on astropy:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 66.18% when pulling b1ea536 on cdeil:heasarc into 2bbb5c4 on astropy:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 66.18% when pulling b1ea536 on cdeil:heasarc into 2bbb5c4 on astropy:master.

@cdeil
Copy link
Member Author

cdeil commented Apr 23, 2015

@leyder, @keflavich and I hacked on this some more, and the basic example of getting a list of available datasets for a given target / mission works.

There's a ton left to do, but maybe this can be done in future pull requests?
The most important thing would be to have a bundled response and local test, so that this is tested on travis-ci. @keflavich can probably help extract and implement this.

Or does someone want to continue working on this one and make a pull request against this one?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.22%) to 66.32% when pulling f1fe7b1 on cdeil:heasarc into bf011db on astropy:master.

Copy link
Member

Choose a reason for hiding this comment

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

The indent here is wrong, and I guess you got the sphinx failure because of that.

@cdeil
Copy link
Member Author

cdeil commented Apr 24, 2015

@abigailStev @kalgash82 @leyder @keflavich I've cleaned this up a bit (removing all the non-implemented methods and tests). If one of you is still working on this and plans to make a PR against this PR, please let me know. If not my suggestion would be to merge this as a bare-bones initial version and then anyone can add to it in new PRs against master in the future.

I didn't include a local test because the response is just a plain FITS file and we currently don't do any processing on it, so there's really no use in putting that in a file in the repo.

@kalgash82
Copy link

@abigailStev @kalgash82 @leyder @keflavich thanks a lot! I can work on it during the next few weeks at some different active levels, but with the idea of having something really useful before the end of May. I don't mind forking your branch and then doing the PR to yours or doing it directly to the astroquery. Whatever is more convenient for you (maybe the second?)

@keflavich
Copy link
Contributor

Is there anything we can test locally? For this first implementation, I guess 'no' - that's OK, even though coveralls will whine.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 65.84% when pulling 745b81e on cdeil:heasarc into b9c6ae6 on astropy:master.

@kalgash82
Copy link

@keflavich, by locally do you mean with no url connection? using a local database? If really important to check, I could make it happen, I think. At least for testing purposes, nothing else.

@keflavich
Copy link
Contributor

@kalgash82 Yes, that's what I mean. The "problem" right now is that everything is so straightforward that we'd be testing more of the monkeypatching than the actual query code.

@cdeil
Copy link
Member Author

cdeil commented Apr 25, 2015

@keflavich @kalgash82 Yes, a local test doesn't make sense here for the moment because there's no parsing or processing going on.

Merging this now ... everyone: if you want to continue working on this, please make a new branch off of astroquery master and make a new pull request.

cdeil added a commit that referenced this pull request Apr 25, 2015
@cdeil cdeil merged commit 7fdb127 into astropy:master Apr 25, 2015
@cdeil cdeil self-assigned this Apr 25, 2015
@abigailStev
Copy link
Contributor

@cdeil Ok sounds good!

@cdeil
Copy link
Member Author

cdeil commented Apr 25, 2015

@kalgash82
Copy link

Awesome!! Thanks @cdeil! I'll be working on giving it more functionality in the next days/weeks unless someone has already done something or thinks that will have more time than me. I will let you know if I need some hand with any trouble :-)

@abigailStev
Copy link
Contributor

Thanks @cdeil! This is a really excellent start! :)

@bsipocz bsipocz removed this from the Wishlist milestone Mar 22, 2017
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.

6 participants