-
-
Notifications
You must be signed in to change notification settings - Fork 390
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.
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
MAST: Cutouts limit #2693
Open
jaymedina
wants to merge
13
commits into
astropy:main
Choose a base branch
from
jaymedina:cutouts-limit
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
MAST: Cutouts limit #2693
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
ce2fbf7
Added Warning message for Tesscut requests when cutout size is too la…
jaymedina c27e3db
Updated documentation to include commentary on TESSCut request timeouts
jaymedina a8b6e5d
Simplifying code, updating unit tests, error prevention
jaymedina 324ac45
removing timeout example, skipping doctest for largequerywarning example
jaymedina 9c0e6af
addressing UnitConversionError for list of scalar inputs converted to…
jaymedina 0422c8e
array-like cutout sizes added to test_tesscut_timeout_param
jaymedina 33f80b3
patched up resetting the default timeout after timeout is adjusted
jaymedina 7e0ff90
asserting that timeout returns to 600s after it is adjusted
jaymedina 8912243
codestyle
jaymedina 3d91932
codestyle
jaymedina c293531
catching all array-like inputs. lists, tuples, np arrays
jaymedina 093989a
resolved zcut unit test error
jaymedina 79e6f05
getting default timeout value directly from conf
jaymedina File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,9 @@ | |
from astropy.table import Table | ||
from astropy.io import fits | ||
|
||
from ..exceptions import InputWarning, NoResultsWarning, InvalidQueryError | ||
from . import conf | ||
from .. import log | ||
from ..exceptions import InputWarning, LargeQueryWarning, NoResultsWarning, InvalidQueryError | ||
|
||
from .utils import parse_input_location | ||
from .core import MastQueryWithLogin | ||
|
@@ -34,7 +36,7 @@ | |
__all__ = ["TesscutClass", "Tesscut", "ZcutClass", "Zcut"] | ||
|
||
|
||
def _parse_cutout_size(size): | ||
def _parse_cutout_size(size, timeout=None, mission=None): | ||
""" | ||
Take a user input cutout size and parse it into the regular format | ||
[ny,nx] where nx/ny are quantities with units either pixels or degrees. | ||
|
@@ -48,6 +50,17 @@ | |
``(ny, nx)`` order. Scalar numbers in ``size`` are assumed to be in | ||
units of pixels. `~astropy.units.Quantity` objects must be in pixel or | ||
angular units. | ||
mission : str, optional | ||
The mission for which the size parsing is being done. This parameter | ||
is mainly meant to trigger a cutout size warning specifically for TESSCut | ||
requests. Default is None. | ||
timeout : int or float, optional | ||
The modified request timeout limit. | ||
The request processing time by default is 600 seconds, meaning an attempt at communicating | ||
with the API will take 600 seconds before timing out. In the context of this function, this | ||
parameter is meant to keep track of whether or not the timeout limit has been modified, which | ||
will affect whether or not a warning message about the cutout size gets triggered. | ||
Default is None. | ||
|
||
Returns | ||
------- | ||
|
@@ -56,35 +69,67 @@ | |
either pixels or degrees. | ||
""" | ||
|
||
# This local variable will change to True if input cutout size exceeds recommended limits for TESS | ||
limit_reached = False | ||
|
||
# Checking 2d size inputs for the recommended cutout size | ||
if (mission == 'TESS') & (not isinstance(size, (int, float, u.Quantity))): | ||
if len(size) == 2: | ||
if np.isscalar(size[0]): | ||
size = [size[0] * u.pixel, size[1] * u.pixel] | ||
|
||
with u.set_enabled_equivalencies(u.pixel_scale(21 * u.arcsec / u.pixel)): | ||
limit_reached = (size * size[0].unit > 30 * u.pixel).any() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here you could put the list into a quantity and then do the check.
So, maybe distinguish in the |
||
|
||
# Making size into an array [ny, nx] | ||
if np.isscalar(size): | ||
jaymedina marked this conversation as resolved.
Show resolved
Hide resolved
|
||
size = np.repeat(size, 2) | ||
|
||
if mission == 'TESS': | ||
limit_reached = (size > 30).any() | ||
|
||
if isinstance(size, u.Quantity): | ||
size = np.atleast_1d(size) | ||
|
||
if len(size) == 1: | ||
size = np.repeat(size, 2) | ||
|
||
# Based on the literature, TESS resolution is approx. 21 arcseconds per pixel. | ||
# We will convert the recommended upper limit for a dimension from pixels | ||
# to the unit being passed. | ||
if mission == 'TESS': | ||
with u.set_enabled_equivalencies(u.pixel_scale(21 * u.arcsec / u.pixel)): | ||
limit_reached = (size > 30 * u.pixel).any() | ||
|
||
if len(size) > 2: | ||
warnings.warn("Too many dimensions in cutout size, only the first two will be used.", | ||
InputWarning) | ||
|
||
# Getting x and y out of the size | ||
|
||
if np.isscalar(size[0]): | ||
x = size[1] | ||
y = size[0] | ||
units = "px" | ||
|
||
elif size[0].unit == u.pixel: | ||
x = size[1].value | ||
y = size[0].value | ||
units = "px" | ||
|
||
elif size[0].unit.physical_type == 'angle': | ||
x = size[1].to(u.deg).value | ||
y = size[0].to(u.deg).value | ||
units = "d" | ||
|
||
else: | ||
raise InvalidQueryError("Cutout size must be in pixels or angular quantity.") | ||
|
||
if (limit_reached) & (not timeout): | ||
warnings.warn("You have selected a large cutout size that may result in a timeout error. We suggest limiting" | ||
" the size of your requested cutout, or changing the request timeout limit from its" | ||
" default 600 seconds to something higher, using the timeout argument.", LargeQueryWarning) | ||
|
||
return {"x": x, "y": y, "units": units} | ||
|
||
|
||
|
@@ -108,6 +153,7 @@ | |
|
||
def get_sectors(self, *, coordinates=None, radius=0*u.deg, product='SPOC', objectname=None, | ||
moving_target=False, mt_type=None): | ||
|
||
""" | ||
Get a list of the TESS data sectors whose footprints intersect | ||
with the given search area. | ||
|
@@ -223,7 +269,8 @@ | |
return Table(sector_dict) | ||
|
||
def download_cutouts(self, *, coordinates=None, size=5, sector=None, product='SPOC', path=".", | ||
inflate=True, objectname=None, moving_target=False, mt_type=None, verbose=False): | ||
inflate=True, objectname=None, moving_target=False, mt_type=None, verbose=False, | ||
timeout=None): | ||
""" | ||
Download cutout target pixel file(s) around the given coordinates with indicated size. | ||
|
||
|
@@ -280,12 +327,24 @@ | |
first majorbody is tried and then smallbody if a matching majorbody is not found. | ||
|
||
NOTE: If moving_target is supplied, this argument is ignored. | ||
timeout : int or float, optional | ||
The modified request timeout limit. | ||
The request processing time by default is 600 seconds, meaning an attempt at communicating | ||
with the API will take 600 seconds before timing out. The timeout upper limit can be modified | ||
using this argument for large cutout requests via TESSCut. Default is None. | ||
|
||
Returns | ||
------- | ||
response : `~astropy.table.Table` | ||
""" | ||
|
||
# Modify TIMEOUT attribute if necessary (usually this is modified for large requests) | ||
if timeout: | ||
default_timeout = conf.timeout | ||
self._service_api_connection.TIMEOUT = timeout | ||
log.info(f"Request timeout upper limit is being changed to {self._service_api_connection.TIMEOUT}" | ||
" seconds.") | ||
|
||
if moving_target: | ||
|
||
# The Moving Targets service is currently only available for SPOC | ||
|
@@ -315,7 +374,7 @@ | |
astrocut_request = f"astrocut?ra={coordinates.ra.deg}&dec={coordinates.dec.deg}" | ||
|
||
# Adding the arguments that are common between moving/still astrocut requests | ||
size_dict = _parse_cutout_size(size) | ||
size_dict = _parse_cutout_size(size, timeout=timeout, mission='TESS') | ||
astrocut_request += f"&y={size_dict['y']}&x={size_dict['x']}&units={size_dict['units']}" | ||
|
||
# Making sure input product is either SPOC or TICA, | ||
|
@@ -356,10 +415,14 @@ | |
os.remove(zipfile_path) | ||
|
||
localpath_table['Local Path'] = [path+x for x in cutout_files] | ||
|
||
if timeout: | ||
self._service_api_connection.TIMEOUT = default_timeout | ||
|
||
return localpath_table | ||
|
||
def get_cutouts(self, *, coordinates=None, size=5, product='SPOC', sector=None, | ||
objectname=None, moving_target=False, mt_type=None): | ||
objectname=None, moving_target=False, mt_type=None, timeout=None): | ||
""" | ||
Get cutout target pixel file(s) around the given coordinates with indicated size, | ||
and return them as a list of `~astropy.io.fits.HDUList` objects. | ||
|
@@ -408,14 +471,26 @@ | |
first majorbody is tried and then smallbody if a matching majorbody is not found. | ||
|
||
NOTE: If moving_target is supplied, this argument is ignored. | ||
timeout : int or float, optional | ||
The modified request timeout limit. | ||
The request processing time by default is 600 seconds, meaning an attempt at communicating | ||
with the API will take 600 seconds before timing out. The timeout upper limit can be modified | ||
using this argument for large cutout requests via TESSCut. Default is None. | ||
|
||
Returns | ||
------- | ||
response : A list of `~astropy.io.fits.HDUList` objects. | ||
""" | ||
|
||
# Modify TIMEOUT attribute if necessary (usually this is modified for large requests) | ||
if timeout: | ||
default_timeout = conf.timeout | ||
self._service_api_connection.TIMEOUT = timeout | ||
log.info(f"Request timeout upper limit is being changed to {self._service_api_connection.TIMEOUT}" | ||
" seconds.") | ||
|
||
# Setting up the cutout size | ||
param_dict = _parse_cutout_size(size) | ||
param_dict = _parse_cutout_size(size, timeout=timeout, mission='TESS') | ||
|
||
# Add sector if present | ||
if sector: | ||
|
@@ -485,6 +560,9 @@ | |
# preserve the original filename in the fits object | ||
cutout_hdus_list[-1].filename = name | ||
|
||
if timeout: | ||
self._service_api_connection.TIMEOUT = default_timeout | ||
|
||
return cutout_hdus_list | ||
|
||
|
||
|
@@ -595,6 +673,7 @@ | |
response : `~astropy.table.Table` | ||
Cutout file(s) for given coordinates | ||
""" | ||
|
||
# Get Skycoord object for coordinates/object | ||
coordinates = parse_input_location(coordinates) | ||
size_dict = _parse_cutout_size(size) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I convert the individual elements in
size
to pixels rather than the whole data structure because unit conversion for the whole data structure happens 3 lines below, so doing that twice for the pixel unit case would result in a unit ofpix^2
which leads to incompatible dimensions being compared. This is the error that comes up:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I would think you don't need the
size[0].unit
in your example and then it would work (you already have that in the screenshot below)