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.

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

Add logging framework #131

Merged
merged 11 commits into from
Oct 21, 2024
31 changes: 19 additions & 12 deletions astrocut/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,33 @@
from ._astropy_init import * # noqa
# ----------------------------------------------------------------------------

# Enforce Python version check during package import.
# This is the same check as the one at the top of setup.py
import sys

__minimum_python_version__ = "3.9"
"""
This module initializes the astrocut package and performs essential setup tasks, including:
- Verifying the version of Python.
- Setting up package-wide logging.
- Importing key modules.
"""

import sys

class UnsupportedPythonError(Exception):
pass

from .exceptions import UnsupportedPythonError
from .utils.logger import setup_logger

if sys.version_info < tuple((int(val) for val in __minimum_python_version__.split('.'))):
raise UnsupportedPythonError("astrocut does not support Python < {}".format(__minimum_python_version__))
# Enforce Python version check during package import.
__minimum_python_version__ = "3.9" # minimum supported Python version
if sys.version_info < tuple(map(int, __minimum_python_version__.split('.'))):
raise UnsupportedPythonError(f"astrocut does not support Python < {__minimum_python_version__}")

# Initialize package-wide logger using astropy's logging system
log = setup_logger()

# Import key submodules and functions if not in setup mode
if not _ASTROPY_SETUP_: # noqa
from .make_cube import CubeFactory, TicaCubeFactory # noqa
from .cube_cut import CutoutFactory # noqa
from .cutouts import fits_cut, img_cut, normalize_img # noqa
from .cutout_processing import (path_to_footprints, center_on_path, # noqa
CutoutsCombiner, build_default_combine_function) # noqa
from .cutout_processing import ( # noqa
path_to_footprints, center_on_path, CutoutsCombiner, build_default_combine_function # noqa
) # noqa
from .asdf_cutouts import asdf_cut, get_center_pixel # noqa
from .footprint_cutouts import cube_cut_from_footprint # noqa
9 changes: 7 additions & 2 deletions astrocut/asdf_cutouts.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
from astropy.coordinates import SkyCoord
from astropy.modeling import models

from . import log
from .utils.utils import _handle_verbose


def _get_cloud_http(s3_uri: Union[str, S3Path], key: str = None, secret: str = None,
token: str = None, verbose: bool = False) -> str:
Expand All @@ -41,8 +44,8 @@ def _get_cloud_http(s3_uri: Union[str, S3Path], key: str = None, secret: str = N
url = f'https://{s3_path.bucket}.s3.amazonaws.com/{s3_path.key}'
resp = requests.head(url, timeout=10)
is_anon = False if resp.status_code == 403 else True
if verbose and not is_anon:
print(f'Attempting to access private S3 bucket: {s3_path.bucket}')
if not is_anon:
log.info(f'Attempting to access private S3 bucket: {s3_path.bucket}')
Copy link
Member

Choose a reason for hiding this comment

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

instead of formatting the string and passing it into the logger, as you have here with the fstring, and below with the str.format() calls, it is best practice to let the logger do the formatting. this is for security (in some places we do call astrocut as a web service, so some of these inputs may be untrusted and we don't want them logged onto a server), but there's another reason as well. If the logging level is skipped (e.g. Debug) the logger can avoid doing any str formatting if it's passed in this way, whereas if you format the string before passing it into the logger, you are always doing that (small) bit of work.

so in this case, my suggestion would be:

Suggested change
log.info(f'Attempting to access private S3 bucket: {s3_path.bucket}')
log.info('Attempting to access private S3 bucket: %s', s3_path.bucket')

And should be applied to all the log formatting here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since AstropyLogger doesn't support lazy formatting, I configured a new logger for astrocut with a similar formatter.


# create file system and get URL of file
fs = s3fs.S3FileSystem(anon=is_anon, key=key, secret=secret, token=token)
Expand Down Expand Up @@ -301,6 +304,8 @@ def asdf_cut(input_file: Union[str, pathlib.Path, S3Path], ra: float, dec: float
astropy.nddata.Cutout2D:
An image cutout object.
"""
# Log messages based on verbosity
_handle_verbose(verbose)

# if file comes from AWS cloud bucket, get HTTP URL to open with asdf
file = input_file
Expand Down
45 changes: 19 additions & 26 deletions astrocut/cube_cut.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
from astropy.io import fits
from astropy.time import Time

from . import __version__
from . import __version__, log
from .exceptions import InputWarning, InvalidQueryError
from .utils.wcs_fitting import fit_wcs_from_points
from .utils.utils import _handle_verbose

# todo: investigate why for small cutouts the astropy version is not working
# from astropy.wcs.utils import fit_wcs_from_points
Expand Down Expand Up @@ -121,8 +122,7 @@ def _parse_table_info(self, table_data, verbose=False):
if data_ind == len(table_data):
raise wcs.NoWcsKeywordsFoundError("No FFI rows contain valid WCS keywords.")

if verbose:
print("Using WCS from row {} out of {}".format(data_ind, len(table_data)))
log.info("Using WCS from row {} out of {}".format(data_ind, len(table_data)))

# Turning the table row into a new header object
wcs_header = fits.header.Header()
Expand Down Expand Up @@ -462,10 +462,9 @@ def _get_cutout(self, transposed_cube, threads: Union[int, Literal["auto"]] = 1,
uncert_cutout = np.pad(uncert_cutout, padding, 'constant', constant_values=np.nan)
aperture = np.pad(aperture, padding[1:], 'constant', constant_values=0)

if verbose:
print("Image cutout cube shape: {}".format(img_cutout.shape))
if self.product == "SPOC":
print("Uncertainty cutout cube shape: {}".format(uncert_cutout.shape))
log.info("Image cutout cube shape: {}".format(img_cutout.shape))
if self.product == "SPOC":
log.info("Uncertainty cutout cube shape: {}".format(uncert_cutout.shape))

return img_cutout, uncert_cutout, aperture

Expand Down Expand Up @@ -844,8 +843,9 @@ def cube_cut(
if unsuccessful returns None.
"""

if verbose:
start_time = time()
start_time = time()
Copy link
Member

Choose a reason for hiding this comment

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

not a huge deal, but we should be using time.monotonic() or time.perf_counter() here instead.

https://docs.python.org/3/library/time.html#time.monotonic

# Log messages based on verbosity
_handle_verbose(verbose)

# Declare the product type being used to make the cutouts
self.product = product.upper()
Expand Down Expand Up @@ -874,9 +874,8 @@ def cube_cut(
else:
self.center_coord = SkyCoord(coordinates, unit='deg')

if verbose:
print("Cutout center coordinate: {},{}".format(self.center_coord.ra.deg,
self.center_coord.dec.deg))
log.info("Cutout center coordinate: {},{}".format(self.center_coord.ra.deg,
self.center_coord.dec.deg))

# Making size into an array [ny, nx]
if np.isscalar(cutout_size):
Expand All @@ -894,10 +893,8 @@ def cube_cut(

# Get cutout limits
self._get_cutout_limits(cutout_size)

if verbose:
print("xmin,xmax: {}".format(self.cutout_lims[1]))
print("ymin,ymax: {}".format(self.cutout_lims[0]))
log.info("xmin,xmax: {}".format(self.cutout_lims[1]))
log.info("ymin,ymax: {}".format(self.cutout_lims[0]))

# Make the cutout
img_cutout, uncert_cutout, aperture = self._get_cutout(getattr(cube[1], cube_data_prop), threads=threads,
Expand All @@ -906,17 +903,15 @@ def cube_cut(
# Get cutout wcs info
cutout_wcs_full = self._get_full_cutout_wcs(cube[2].header)
max_dist, sigma = self._fit_cutout_wcs(cutout_wcs_full, img_cutout.shape[1:])
if verbose:
print("Maximum distance between approximate and true location: {}".format(max_dist))
print("Error in approximate WCS (sigma): {}".format(sigma))
log.info("Maximum distance between approximate and true location: {}".format(max_dist))
log.info("Error in approximate WCS (sigma): {}".format(sigma))

cutout_wcs_dict = self._get_cutout_wcs_dict()

# Build the TPF
tpf_object = self._build_tpf(cube, img_cutout, uncert_cutout, cutout_wcs_dict, aperture)

if verbose:
write_time = time()
write_time = time()

if not target_pixel_file:
_, flename = os.path.split(cube_file)
Expand All @@ -931,8 +926,7 @@ def cube_cut(
target_pixel_file = os.path.join(output_path, target_pixel_file)


if verbose:
print("Target pixel file: {}".format(target_pixel_file))
log.info("Target pixel file: {}".format(target_pixel_file))

# Make sure the output directory exists
if not os.path.exists(output_path):
Expand All @@ -941,8 +935,7 @@ def cube_cut(
# Write the TPF
tpf_object.writeto(target_pixel_file, overwrite=True, checksum=True)

if verbose:
print("Write time: {:.2} sec".format(time()-write_time))
print("Total time: {:.2} sec".format(time()-start_time))
log.info("Write time: {:.2} sec".format(time()-write_time))
log.info("Total time: {:.2} sec".format(time()-start_time))

return target_pixel_file
10 changes: 7 additions & 3 deletions astrocut/cutout_processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

from scipy.interpolate import splprep, splev

from .utils.utils import get_fits
from . import log
from .utils.utils import get_fits, _handle_verbose
from .exceptions import DataWarning, InvalidInputError


Expand Down Expand Up @@ -148,14 +149,15 @@ def _moving_target_focus(path, size, cutout_fles, verbose=False):
response : `~astropy.table.Table`
New cutout table.
"""
# Log messages based on verbosity
_handle_verbose(verbose)

cutout_table_list = list()

tck_tuple, u = splprep([path["position"].ra, path["position"].dec], u=path["time"].jd, s=0)

for fle in cutout_fles:
if verbose:
print(fle)
log.info(f'Processing file: {fle}')

# Get the stuff we need from the cutout file
hdu = fits.open(fle)
Expand Down Expand Up @@ -373,6 +375,8 @@ def center_on_path(path, size, cutout_fles, target=None, img_wcs=None,
"""

# TODO: add ability to take sizes like in rest of cutout functionality
# Log messages based on verbosity
_handle_verbose(verbose)

# Performing the path transformation
cutout_table = _moving_target_focus(path, size, cutout_fles, verbose)
Expand Down
Loading
Loading