Skip to content

Conversation

@keflavich
Copy link
Contributor

NASA ADS querying was broken because of incorrectly used modules in its core.
There are still no local tests, but at least now it uses BaseQuery rather than
commons.

@bsipocz
Copy link
Member

bsipocz commented Oct 26, 2015

@keflavich - speaking of nasa_ads, is #20 still relevant? There is a test in there as a starter, etc.

@keflavich
Copy link
Contributor Author

Wow, yes, it looks like that has been sitting there stagnant for a long time. It should probably replace the current module, but it needs review & some testing before that can happen.

@keflavich
Copy link
Contributor Author

#20 should override this at some point, but I'm going to merge it because it fixes a failing remote test. Other than this, the only failures I have gotten today are timeout errors.

keflavich added a commit that referenced this pull request Oct 26, 2015
@keflavich keflavich merged commit e70f3ac into astropy:master Oct 26, 2015
@keflavich keflavich deleted the fix_nasaads_remote branch October 26, 2015 15:30
@bsipocz
Copy link
Member

bsipocz commented Oct 26, 2015

👍

@bsipocz
Copy link
Member

bsipocz commented Oct 26, 2015

btw have you seen @andycasey's relatively new ads tool? I wonder whether he is interested in making it part of astroquery, or how compatible it is with our query tools.

@keflavich
Copy link
Contributor Author

yeah, I use it - it's been around for awhile in some other forms. I asked him about this in September 2013, and he expressed interest/excitement, but I think nothing ever came of it.

@andycasey
Copy link
Member

Yeah, I failed hard on that.

The code is much more mature now, and half of me was reluctant to move things into astroquery before ADS had updated their API (which was in the works for some time).

I still think it would be advantageous to have ADS part of astroquery, but without having used astroquery, would the "customer usage" be similar? Right now nasa_ads parses the response as an AstroPy Table, but in the ads library we instantiate each article as an object so that we can easily build reference and citation trees to other articles. Would it go against the astroquery mantra if the response returned from a query was some list (generator, actually) of Article objects, each with their own attributes?

@keflavich
Copy link
Contributor Author

@andycasey No, I think it would be useful to have that interface. But it would probably also be useful to parse into a table, because sometimes tables are nice, no?

@andycasey
Copy link
Member

That's good to know! Yeah, tables are great, some of my best friends are tables.

I'll work on getting this into a PR-able form before Christmas.

@bsipocz
Copy link
Member

bsipocz commented Oct 26, 2015

Sounds good. Thanks @andycasey

@keflavich
Copy link
Contributor Author

@andycasey Christmas is soon, any progress? Would still be great to have your module as part of astroquery.

@bsipocz
Copy link
Member

bsipocz commented Dec 14, 2015

@keflavich I've offered Andy to help out if needed, but I will be back in the UK only after Christmas.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants