Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get IBE's query_region_async function minimally working #1430

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Get IBE's query_region_async function minimally working #1430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7fc6dbd
Fix bug in list_missions()
odysseus9672 Apr 25, 2019
ff4988c
break a long line
odysseus9672 Apr 25, 2019
75af6f7
remove redundant backslash (PEP 8)
odysseus9672 Apr 25, 2019
04902fc
Fix placement of rstrip
odysseus9672 Apr 25, 2019
0766865
fix whitespace PEP 8 issues
odysseus9672 Apr 25, 2019
0c4c7c7
Fix ibe's list_datasets, list_tables, and get_columns methods
odysseus9672 Apr 25, 2019
516bc5e
remove extraneous comment
odysseus9672 Apr 25, 2019
6c61f32
get query_region_async minimally functional.
odysseus9672 Apr 25, 2019
a17f56b
Fix PEP8 issues
odysseus9672 Apr 25, 2019
62ce28b
Remove version checking hack
odysseus9672 Apr 26, 2019
51f2abe
Remove `dataset` parameter from `config`.
odysseus9672 Apr 26, 2019
b505817
Trim code
odysseus9672 Apr 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions astroquery/ibe/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,19 @@ class Conf(_config.ConfigNamespace):
Configuration parameters for `astroquery.ibe`.
"""

# For some reason the IBE in the URL is case sensitive
server = _config.ConfigItem(
'http://irsa.ipac.caltech.edu/ibe/',
'http://irsa.ipac.caltech.edu/IBE/',
'Name of the IBE server to use.')
mission = _config.ConfigItem(
'ptf',
('Default mission. See, for example, '
'http://irsa.ipac.caltech.edu/ibe/search/ for options.'))

dataset = _config.ConfigItem(
'images',
('Default data set. See, for example, '
'http://irsa.ipac.caltech.edu/ibe/search/ptf for options.'))
table = _config.ConfigItem(
'level1',
('Default table. See, for example, '
'http://irsa.ipac.caltech.edu/ibe/search/ptf/images for options.'))
'ptf.ptf_procimg',
('Default table. Select the desired mission at '
'http://irsa.ipac.caltech.edu/ibe/search/ for options.'))
timeout = _config.ConfigItem(
60,
'Time limit for connecting to the IRSA server.')
Expand Down
136 changes: 41 additions & 95 deletions astroquery/ibe/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
from __future__ import print_function, division

import os

import webbrowser
from bs4 import BeautifulSoup

import astropy.coordinates as coord
from astropy.table import Table
from astropy.io.ascii.html import HTML
import six

from ..exceptions import InvalidQueryError
Expand All @@ -24,16 +26,14 @@

__all__ = ['Ibe', 'IbeClass']


class IbeClass(BaseQuery):
URL = conf.server
MISSION = conf.mission
DATASET = conf.dataset
TABLE = conf.table
TIMEOUT = conf.timeout

def query_region(
self, coordinate=None, where=None, mission=None, dataset=None,
Copy link
Member

Choose a reason for hiding this comment

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

Removing kwargs, even if they are unused, should be done via a deprecation.
There is a decorator for doing that in astropy, and it was backported to the LTS release (2.0.12), too, but not to 3.0.x, so I suppose we shouldn't yet use it here.
@keflavich - what do you think? In astroML I worked around this very same issue by copying the decorator to astroML.utils and importing it from there when the astropy version is not the preferred one (we also do similar things in photutils to work around issues that are not available in all supported versions of astropy).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm onboard with using those decorators, but in this case, I think these kwargs are being removed because they didn't do anything? But your point below that it's breaking the tests suggests that yes, we should use the deprecation and copy over the deprecator decorator.

self, coordinate=None, where=None, mission=None,
table=None, columns=None, width=None, height=None,
intersect='OVERLAPS', most_centered=False):
"""
Expand All @@ -56,10 +56,6 @@ def query_region(
`~astropy.coordinates.SkyCoord`. Required if ``where`` is absent.
where : str
SQL-like query string. Required if ``coordinates`` is absent.
mission : str
The mission to be used (if not the default mission).
dataset : str
The dataset to be used (if not the default dataset).
table : str
The table to be queried (if not the default table).
columns : str, list
Expand Down Expand Up @@ -100,26 +96,27 @@ def query_region(
A table containing the results of the query
"""
response = self.query_region_async(
coordinate=coordinate, where=where, mission=mission,
dataset=dataset, table=table, columns=columns, width=width,
coordinate=coordinate, where=where,
table=table, columns=columns, width=width,
height=height, intersect=intersect, most_centered=most_centered)

# Raise exception, if request failed
response.raise_for_status()

return Table.read(response.text, format='ipac', guess=False)
return commons.parse_votable(
response.content).get_first_table().to_table()

def query_region_sia(self, coordinate=None, mission=None,
dataset=None, table=None, width=None,
table=None, width=None,
height=None, intersect='OVERLAPS',
most_centered=False):
"""
Query using simple image access protocol. See ``query_region`` for
details. The returned table will include a list of URLs.
"""
response = self.query_region_async(
coordinate=coordinate, mission=mission,
dataset=dataset, table=table, width=width,
coordinate=coordinate,
table=table, width=width,
height=height, intersect=intersect, most_centered=most_centered,
action='sia')

Expand All @@ -130,7 +127,7 @@ def query_region_sia(self, coordinate=None, mission=None,
response.text).get_first_table().to_table()

def query_region_async(
self, coordinate=None, where=None, mission=None, dataset=None,
self, coordinate=None, where=None,
table=None, columns=None, width=None, height=None,
action='search',
intersect='OVERLAPS', most_centered=False):
Expand All @@ -154,10 +151,6 @@ def query_region_async(
`~astropy.coordinates.SkyCoord`. Required if ``where`` is absent.
where : str
SQL-like query string. Required if ``coordinates`` is absent.
mission : str
The mission to be used (if not the default mission).
dataset : str
The dataset to be used (if not the default dataset).
table : str
The table to be queried (if not the default table).
columns : str, list
Expand Down Expand Up @@ -195,7 +188,8 @@ def query_region_async(
The action to perform at the server. The default is ``'search'``,
which returns a table of the available data. ``'data'`` requires
advanced path construction that is not yet supported. ``'sia'``
provides access to the 'simple image access' IVOA protocol
is here for legacy reasons. This function is built around IRSA's
VO TAP API. IRSA's SIA API is completely different.

Returns
-------
Expand All @@ -220,8 +214,16 @@ def query_region_async(
"The action='data' option is a placeholder for future " +
"functionality.")

if action == "sia":
raise NotImplementedError(
"The action='sia' is not implemented for IRSA's IBE "
"interface because IRSA's SIA interface is radically "
"different.")

args = {
'INTERSECT': intersect
'INTERSECT': intersect,
'TABLE': table or self.TABLE,
'RESPONSEFORMAT': "VOTABLE"
}

# Note: in IBE, if 'mcen' argument is present, it is true.
Expand All @@ -247,14 +249,11 @@ def query_region_async(
columns = columns.split()
args['columns'] = ','.join(columns)

url = "{URL}{action}/{mission}/{dataset}/{table}".format(
URL=self.URL,
action=action,
mission=mission or self.MISSION,
dataset=dataset or self.DATASET,
table=table or self.TABLE)
url = self.URL.rstrip("/")

response = self._request('GET', url, args, timeout=self.TIMEOUT)

return self._request('GET', url, args, timeout=self.TIMEOUT)
return response

def list_missions(self, cache=True):
"""
Expand All @@ -276,53 +275,17 @@ def list_missions(self, cache=True):

root = BeautifulSoup(response.text)
links = root.findAll('a')
missions = [os.path.basename(a.attrs['href']) for a in links]

missions = [os.path.basename(a.attrs['href'].rstrip('/'))
for a in links]
self._missions = missions

return missions

def list_datasets(self, mission=None, cache=True):
def list_tables(self, mission=None, cache=True):
"""
For a given mission, list the available datasets

Parameters
----------
mission : str
A mission name. Must be one of the valid missions from
`~astroquery.ibe.IbeClass.list_missions`. Defaults to the
configured Mission
cache : bool
Cache the query result

Returns
-------
datasets : list
A list of dataset names
"""
if mission is None:
mission = self.MISSION
if mission not in self.list_missions():
raise ValueError("Invalid mission specified: {0}."
"Must be one of: {1}"
.format(mission, self.list_missions()))

url = "{URL}search/{mission}/".format(URL=self.URL, mission=mission)
response = self._request('GET', url, timeout=self.TIMEOUT,
cache=cache)

root = BeautifulSoup(response.text)
links = root.findAll('a')
datasets = [a.text for a in links
if a.attrs['href'].count('/') >= 4 # shown as '..'; ignore
]

return datasets

def list_tables(self, mission=None, dataset=None, cache=True):
"""
For a given mission and dataset (see
`~.astroquery.ibe.IbeClass.list_missions`,
`~astroquery.ibe.IbeClass.list_datasets`), return the list of valid
For a given mission (see
`~.astroquery.ibe.IbeClass.list_missions`), return the list of valid
table names to query.

Parameters
Expand All @@ -331,9 +294,6 @@ def list_tables(self, mission=None, dataset=None, cache=True):
A mission name. Must be one of the valid missions from
`~.astroquery.ibe.IbeClass.list_missions`. Defaults to the
configured Mission
dataset : str
A dataset name. Must be one of the valid dataset from
``list_datsets(mission)``. Defaults to the configured Dataset
cache : bool
Cache the query result

Expand All @@ -344,23 +304,13 @@ def list_tables(self, mission=None, dataset=None, cache=True):
"""
if mission is None:
mission = self.MISSION
if dataset is None:
dataset = self.DATASET

if mission not in self.list_missions():
raise ValueError("Invalid mission specified: {0}."
"Must be one of: {1}"
.format(mission, self.list_missions()))

if dataset not in self.list_datasets(mission, cache=cache):
raise ValueError("Invalid dataset {0} specified for mission {1}."
"Must be one of: {2}"
.format(dataset, mission,
self.list_datasets(mission, cache=True)))

url = "{URL}search/{mission}/{dataset}/".format(URL=self.URL,
mission=mission,
dataset=dataset)
url = "{URL}search/{mission}/".format(URL=self.URL, mission=mission)
response = self._request('GET', url, timeout=self.TIMEOUT,
cache=cache)

Expand All @@ -372,38 +322,33 @@ def list_tables(self, mission=None, dataset=None, cache=True):
# def get_data(self, **kwargs):
# return self.query_region_async(retrieve_data=True, **kwargs)

def show_docs(self, mission=None, dataset=None, table=None):
def show_docs(self, mission=None, table=None):
"""
Open the documentation for a given table in a web browser.

Parameters
----------
mission : str
The mission to be used (if not the default mission).
dataset : str
The dataset to be used (if not the default dataset).
table : str
The table to be queried (if not the default table).
"""

url = "{URL}docs/{mission}/{dataset}/{table}".format(
url = "{URL}docs/{mission}/{table}".format(
URL=self.URL,
mission=mission or self.MISSION,
dataset=dataset or self.DATASET,
table=table or self.TABLE)

return webbrowser.open(url)

def get_columns(self, mission=None, dataset=None, table=None):
def get_columns(self, mission=None, table=None):
"""
Get the schema for a given table.

Parameters
----------
mission : str
The mission to be used (if not the default mission).
dataset : str
The dataset to be used (if not the default dataset).
table : str
The table to be queried (if not the default table).

Expand All @@ -413,19 +358,20 @@ def get_columns(self, mission=None, dataset=None, table=None):
A table containing a description of the columns
"""

url = "{URL}search/{mission}/{dataset}/{table}".format(
url = "{URL}search/{mission}/{table}/".format(
URL=self.URL,
mission=mission or self.MISSION,
dataset=dataset or self.DATASET,
table=table or self.TABLE)

response = self._request(
'GET', url, {'FORMAT': 'METADATA'}, timeout=self.TIMEOUT)
'GET', url, {'FORMAT': 'METADATA'},
timeout=self.TIMEOUT)

# Raise exception, if request failed
response.raise_for_status()
HTMLtoTable = HTML()
Copy link
Member

Choose a reason for hiding this comment

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

It would be nicer to use the Table initializer directly


return Table.read(response.text, format='ipac', guess=False)
return HTMLtoTable.read(response.text)


Ibe = IbeClass()