From 203188ea08a49bf4a0c3e7dd75dcc5c4e8509948 Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Tue, 19 Aug 2025 10:06:10 -0700 Subject: [PATCH 1/7] use LOG.isEnabledFor(DEBUG) --- mapillary_tools/geotag/base.py | 8 ++++++-- mapillary_tools/geotag/factory.py | 2 +- mapillary_tools/geotag/geotag_images_from_exiftool.py | 2 +- mapillary_tools/geotag/geotag_videos_from_exiftool.py | 2 +- mapillary_tools/geotag/utils.py | 2 +- mapillary_tools/http.py | 4 ++-- mapillary_tools/process_geotag_properties.py | 2 +- mapillary_tools/sample_video.py | 3 +-- mapillary_tools/upload.py | 6 +++--- 9 files changed, 17 insertions(+), 14 deletions(-) diff --git a/mapillary_tools/geotag/base.py b/mapillary_tools/geotag/base.py index f4384fa91..406c26fc3 100644 --- a/mapillary_tools/geotag/base.py +++ b/mapillary_tools/geotag/base.py @@ -46,7 +46,7 @@ def to_description( map_results, desc="Extracting images", unit="images", - disable=LOG.getEffectiveLevel() <= logging.DEBUG, + disable=LOG.isEnabledFor(logging.DEBUG), total=len(extractors), ) ) @@ -62,6 +62,8 @@ def run_extraction(cls, extractor: TImageExtractor) -> types.ImageMetadataOrErro try: return extractor.extract() except exceptions.MapillaryDescriptionError as ex: + if LOG.isEnabledFor(logging.DEBUG): + LOG.error(f"Error {cls.__name__}({image_path.name}): {ex}") return types.describe_error_metadata( ex, image_path, filetype=types.FileType.IMAGE ) @@ -112,7 +114,7 @@ def to_description( map_results, desc="Extracting videos", unit="videos", - disable=LOG.getEffectiveLevel() <= logging.DEBUG, + disable=LOG.isEnabledFor(logging.DEBUG), total=len(extractors), ) ) @@ -128,6 +130,8 @@ def run_extraction(cls, extractor: TVideoExtractor) -> types.VideoMetadataOrErro try: return extractor.extract() except exceptions.MapillaryDescriptionError as ex: + if LOG.isEnabledFor(logging.DEBUG): + LOG.error(f"Error {cls.__name__}({video_path.name}): {ex}") return types.describe_error_metadata( ex, video_path, filetype=types.FileType.VIDEO ) diff --git a/mapillary_tools/geotag/factory.py b/mapillary_tools/geotag/factory.py index c99766639..0a226b033 100644 --- a/mapillary_tools/geotag/factory.py +++ b/mapillary_tools/geotag/factory.py @@ -67,7 +67,7 @@ def process( reprocessable_paths = set(paths) for idx, option in enumerate(options): - if LOG.getEffectiveLevel() <= logging.DEBUG: + if LOG.isEnabledFor(logging.DEBUG): LOG.info( f"==> Processing {len(reprocessable_paths)} files with source {option}..." ) diff --git a/mapillary_tools/geotag/geotag_images_from_exiftool.py b/mapillary_tools/geotag/geotag_images_from_exiftool.py index 6c74e0a99..452bfecd9 100644 --- a/mapillary_tools/geotag/geotag_images_from_exiftool.py +++ b/mapillary_tools/geotag/geotag_images_from_exiftool.py @@ -93,7 +93,7 @@ def _generate_image_extractors( LOG.warning( "Failed to parse ExifTool XML: %s", str(ex), - exc_info=LOG.getEffectiveLevel() <= logging.DEBUG, + exc_info=LOG.isEnabledFor(logging.DEBUG), ) rdf_by_path = {} else: diff --git a/mapillary_tools/geotag/geotag_videos_from_exiftool.py b/mapillary_tools/geotag/geotag_videos_from_exiftool.py index 04bbef4c5..19256cef6 100644 --- a/mapillary_tools/geotag/geotag_videos_from_exiftool.py +++ b/mapillary_tools/geotag/geotag_videos_from_exiftool.py @@ -110,7 +110,7 @@ def _generate_video_extractors( LOG.warning( "Failed to parse ExifTool XML: %s", str(ex), - exc_info=LOG.getEffectiveLevel() <= logging.DEBUG, + exc_info=LOG.isEnabledFor(logging.DEBUG), ) rdf_by_path = {} else: diff --git a/mapillary_tools/geotag/utils.py b/mapillary_tools/geotag/utils.py index ef311e437..8c855914b 100644 --- a/mapillary_tools/geotag/utils.py +++ b/mapillary_tools/geotag/utils.py @@ -46,7 +46,7 @@ def index_rdf_description_by_path( try: etree = ET.parse(xml_path) except ET.ParseError as ex: - verbose = LOG.getEffectiveLevel() <= logging.DEBUG + verbose = LOG.isEnabledFor(logging.DEBUG) if verbose: LOG.warning("Failed to parse %s", xml_path, exc_info=True) else: diff --git a/mapillary_tools/http.py b/mapillary_tools/http.py index 3f0721207..2a384eb1a 100644 --- a/mapillary_tools/http.py +++ b/mapillary_tools/http.py @@ -88,7 +88,7 @@ def _log_debug_request(self, method: str | bytes, url: str | bytes, **kwargs): if self.disable_logging_request: return - if logging.getLogger().getEffectiveLevel() <= logging.DEBUG: + if not LOG.isEnabledFor(logging.DEBUG): return if isinstance(method, str) and isinstance(url, str): @@ -124,7 +124,7 @@ def _log_debug_response(self, resp: requests.Response): if self.disable_logging_response: return - if logging.getLogger().getEffectiveLevel() <= logging.DEBUG: + if not LOG.isEnabledFor(logging.DEBUG): return elapsed = resp.elapsed.total_seconds() * 1000 # Convert to milliseconds diff --git a/mapillary_tools/process_geotag_properties.py b/mapillary_tools/process_geotag_properties.py index 0c1236194..d22901214 100644 --- a/mapillary_tools/process_geotag_properties.py +++ b/mapillary_tools/process_geotag_properties.py @@ -167,7 +167,7 @@ def _overwrite_exif_tags( metadatas, desc="Overwriting EXIF", unit="images", - disable=LOG.getEffectiveLevel() <= logging.DEBUG, + disable=LOG.isEnabledFor(logging.DEBUG), ): dt = datetime.datetime.fromtimestamp(metadata.time, datetime.timezone.utc) dt = dt.replace(tzinfo=datetime.timezone.utc) diff --git a/mapillary_tools/sample_video.py b/mapillary_tools/sample_video.py index 3053e8bea..d6bfb0b5d 100644 --- a/mapillary_tools/sample_video.py +++ b/mapillary_tools/sample_video.py @@ -125,12 +125,11 @@ def sample_video( except Exception as ex: if skip_sample_errors: - exc_info = LOG.getEffectiveLevel() <= logging.DEBUG LOG.warning( "Skipping the error sampling %s: %s", video_path, str(ex), - exc_info=exc_info, + exc_info=LOG.isEnabledFor(logging.DEBUG), ) else: raise diff --git a/mapillary_tools/upload.py b/mapillary_tools/upload.py index 548c0d451..4a31ad84c 100644 --- a/mapillary_tools/upload.py +++ b/mapillary_tools/upload.py @@ -152,7 +152,7 @@ def zip_images(import_path: Path, zip_dir: Path, desc_path: str | None = None): def log_exception(ex: Exception) -> None: - if LOG.getEffectiveLevel() <= logging.DEBUG: + if LOG.isEnabledFor(logging.DEBUG): exc_info = ex else: exc_info = None @@ -280,7 +280,7 @@ def upload_start(payload: uploader.Progress) -> None: unit_scale=True, unit_divisor=1024, initial=payload.get("offset", 0), - disable=LOG.getEffectiveLevel() <= logging.DEBUG, + disable=LOG.isEnabledFor(logging.DEBUG), ) @emitter.on("upload_fetch_offset") @@ -338,7 +338,7 @@ def upload_fetch_offset(payload: uploader.Progress) -> None: def upload_progress(payload: uploader.Progress): type: uploader.EventName = "upload_progress" - if LOG.getEffectiveLevel() <= logging.DEBUG: + if LOG.isEnabledFor(logging.DEBUG): # In debug mode, we want to see the progress every 30 seconds # instead of every chunk (which is too verbose) INTERVAL_SECONDS = 30 From f463b3c12744b3cedc9c7d5721824ecc4b1e6d55 Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Wed, 20 Aug 2025 21:54:20 -0700 Subject: [PATCH 2/7] configure logger in multiprocessing --- mapillary_tools/commands/__main__.py | 16 +++------- mapillary_tools/exif_read.py | 3 +- mapillary_tools/geotag/base.py | 4 +-- mapillary_tools/utils.py | 46 +++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 16 deletions(-) diff --git a/mapillary_tools/commands/__main__.py b/mapillary_tools/commands/__main__.py index 929a454f6..e0466cd22 100644 --- a/mapillary_tools/commands/__main__.py +++ b/mapillary_tools/commands/__main__.py @@ -8,6 +8,7 @@ from .. import api_v4, constants, exceptions, VERSION from ..upload import log_exception +from ..utils import configure_logger, get_app_name from . import ( authenticate, process, @@ -31,8 +32,8 @@ ] -# do not use __name__ here is because if you run tools as a module, __name__ will be "__main__" -LOG = logging.getLogger("mapillary_tools") +# Root logger of mapillary_tools (not including third-party libraries) +LOG = logging.getLogger(get_app_name()) # Handle shared arguments/options here @@ -79,13 +80,6 @@ def add_general_arguments(parser, command): ) -def configure_logger(logger: logging.Logger, stream=None) -> None: - formatter = logging.Formatter("%(asctime)s - %(levelname)-7s - %(message)s") - handler = logging.StreamHandler(stream) - handler.setFormatter(formatter) - logger.addHandler(handler) - - def _log_params(argvars: dict) -> None: MAX_ENTRIES = 5 @@ -152,9 +146,7 @@ def main(): args = parser.parse_args() - log_level = logging.DEBUG if args.verbose else logging.INFO - configure_logger(LOG, sys.stderr) - LOG.setLevel(log_level) + configure_logger(LOG, level=logging.DEBUG if args.verbose else logging.INFO) LOG.debug("%s", version_text) argvars = vars(args) diff --git a/mapillary_tools/exif_read.py b/mapillary_tools/exif_read.py index f0f3894ce..67536ab7a 100644 --- a/mapillary_tools/exif_read.py +++ b/mapillary_tools/exif_read.py @@ -871,7 +871,8 @@ def __init__(self, path_or_stream: Path | T.BinaryIO) -> None: def _xmp_with_reason(self, reason: str) -> ExifReadFromXMP | None: if not self._xml_extracted: - LOG.debug('Extracting XMP for "%s"', reason) + # TODO Disabled because too verbose but still useful to know + # LOG.debug('Extracting XMP for "%s"', reason) self._cached_xml = self._extract_xmp() self._xml_extracted = True diff --git a/mapillary_tools/geotag/base.py b/mapillary_tools/geotag/base.py index 406c26fc3..0a43995c3 100644 --- a/mapillary_tools/geotag/base.py +++ b/mapillary_tools/geotag/base.py @@ -63,7 +63,7 @@ def run_extraction(cls, extractor: TImageExtractor) -> types.ImageMetadataOrErro return extractor.extract() except exceptions.MapillaryDescriptionError as ex: if LOG.isEnabledFor(logging.DEBUG): - LOG.error(f"Error {cls.__name__}({image_path.name}): {ex}") + LOG.error(f"{cls.__name__}({image_path.name}): {ex}") return types.describe_error_metadata( ex, image_path, filetype=types.FileType.IMAGE ) @@ -131,7 +131,7 @@ def run_extraction(cls, extractor: TVideoExtractor) -> types.VideoMetadataOrErro return extractor.extract() except exceptions.MapillaryDescriptionError as ex: if LOG.isEnabledFor(logging.DEBUG): - LOG.error(f"Error {cls.__name__}({video_path.name}): {ex}") + LOG.error(f"{cls.__name__}({video_path.name}): {ex}") return types.describe_error_metadata( ex, video_path, filetype=types.FileType.VIDEO ) diff --git a/mapillary_tools/utils.py b/mapillary_tools/utils.py index 6d5f82b39..ae74bd0ed 100644 --- a/mapillary_tools/utils.py +++ b/mapillary_tools/utils.py @@ -1,6 +1,7 @@ from __future__ import annotations import hashlib +import logging import os import typing as T from multiprocessing import Pool @@ -223,5 +224,48 @@ def mp_map_maybe( if disable_multiprocessing: yield from map(func, iterable) else: - with Pool(processes=pool_num_processes) as pool: + app_logger = logging.getLogger(get_app_name()) + + with Pool( + initializer=configure_logger, + initargs=(None, app_logger.getEffectiveLevel()), + processes=pool_num_processes, + ) as pool: yield from pool.imap(func, iterable) + + +def configure_logger( + logger: logging.Logger | None = None, level: int = logging.INFO +) -> logging.Logger: + """Configure logging in each worker process""" + if logger is None: + # Root logger if app name is "" + logger = logging.getLogger(get_app_name()) + + logger.setLevel(level) + + try: + raise ImportError # Disable for now + from rich.console import Console + from rich.logging import RichHandler + except ImportError: + formatter = logging.Formatter( + "%(asctime)s.%(msecs)03d - %(levelname)-7s - %(message)s", + datefmt="%H:%M:%S", + ) + handler: logging.Handler = logging.StreamHandler() + handler.setFormatter(formatter) + else: + handler = RichHandler(console=Console(stderr=True), rich_tracebacks=True) + + logger.addHandler(handler) + + return logger + + +def get_app_name() -> str: + if __name__: + return __name__.split(".")[0] + else: + # Rare case + return "" From ba808834c76de37ba6f408dae8db5aa3bb38cdab Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Wed, 20 Aug 2025 21:57:26 -0700 Subject: [PATCH 3/7] fix comments --- mapillary_tools/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mapillary_tools/utils.py b/mapillary_tools/utils.py index ae74bd0ed..d818d1e24 100644 --- a/mapillary_tools/utils.py +++ b/mapillary_tools/utils.py @@ -245,7 +245,8 @@ def configure_logger( logger.setLevel(level) try: - raise ImportError # Disable for now + # Disable globally for now. TODO Disable it in non-interactive mode only + raise ImportError from rich.console import Console from rich.logging import RichHandler except ImportError: From f3711a263e741ed66d0db4f687e42ff0f32769db Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Wed, 20 Aug 2025 22:00:10 -0700 Subject: [PATCH 4/7] fix types --- mapillary_tools/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mapillary_tools/utils.py b/mapillary_tools/utils.py index d818d1e24..f9e8ac40c 100644 --- a/mapillary_tools/utils.py +++ b/mapillary_tools/utils.py @@ -247,8 +247,8 @@ def configure_logger( try: # Disable globally for now. TODO Disable it in non-interactive mode only raise ImportError - from rich.console import Console - from rich.logging import RichHandler + from rich.console import Console # type: ignore + from rich.logging import RichHandler # type: ignore except ImportError: formatter = logging.Formatter( "%(asctime)s.%(msecs)03d - %(levelname)-7s - %(message)s", @@ -257,7 +257,7 @@ def configure_logger( handler: logging.Handler = logging.StreamHandler() handler.setFormatter(formatter) else: - handler = RichHandler(console=Console(stderr=True), rich_tracebacks=True) + handler = RichHandler(console=Console(stderr=True), rich_tracebacks=True) # type: ignore logger.addHandler(handler) From b6f349e44b6268f45daceb094e2f0cb30e83fd96 Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Wed, 20 Aug 2025 22:16:23 -0700 Subject: [PATCH 5/7] use ProcessPoolExecutor --- mapillary_tools/utils.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/mapillary_tools/utils.py b/mapillary_tools/utils.py index f9e8ac40c..21bba02dd 100644 --- a/mapillary_tools/utils.py +++ b/mapillary_tools/utils.py @@ -4,7 +4,7 @@ import logging import os import typing as T -from multiprocessing import Pool +from concurrent.futures import as_completed, ProcessPoolExecutor from pathlib import Path @@ -215,23 +215,24 @@ def mp_map_maybe( num_processes: int | None = None, ) -> T.Generator[TMapOut, None, None]: if num_processes is None: - pool_num_processes = None + max_workers = None disable_multiprocessing = False else: - pool_num_processes = max(num_processes, 1) + max_workers = max(num_processes, 1) disable_multiprocessing = num_processes <= 0 if disable_multiprocessing: yield from map(func, iterable) else: app_logger = logging.getLogger(get_app_name()) - - with Pool( + with ProcessPoolExecutor( + max_workers=max_workers, initializer=configure_logger, initargs=(None, app_logger.getEffectiveLevel()), - processes=pool_num_processes, - ) as pool: - yield from pool.imap(func, iterable) + ) as executor: + futures = [executor.submit(func, item) for item in iterable] + for future in as_completed(futures): + yield future.result() def configure_logger( From 21165cca076575caf78694affcf6dc6502ac4c18 Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Wed, 20 Aug 2025 22:19:45 -0700 Subject: [PATCH 6/7] imports --- mapillary_tools/uploader.py | 1 - mapillary_tools/utils.py | 7 ++++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mapillary_tools/uploader.py b/mapillary_tools/uploader.py index d23c25498..1b1effc50 100644 --- a/mapillary_tools/uploader.py +++ b/mapillary_tools/uploader.py @@ -1,7 +1,6 @@ from __future__ import annotations import concurrent.futures - import dataclasses import io import json diff --git a/mapillary_tools/utils.py b/mapillary_tools/utils.py index 21bba02dd..36a48b057 100644 --- a/mapillary_tools/utils.py +++ b/mapillary_tools/utils.py @@ -1,10 +1,11 @@ from __future__ import annotations +import concurrent.futures + import hashlib import logging import os import typing as T -from concurrent.futures import as_completed, ProcessPoolExecutor from pathlib import Path @@ -225,13 +226,13 @@ def mp_map_maybe( yield from map(func, iterable) else: app_logger = logging.getLogger(get_app_name()) - with ProcessPoolExecutor( + with concurrent.futures.ProcessPoolExecutor( max_workers=max_workers, initializer=configure_logger, initargs=(None, app_logger.getEffectiveLevel()), ) as executor: futures = [executor.submit(func, item) for item in iterable] - for future in as_completed(futures): + for future in concurrent.futures.as_completed(futures): yield future.result() From d7fd4ccb2a3d4cc069ef87b68a388c45e3ec2deb Mon Sep 17 00:00:00 2001 From: Tao Peng Date: Wed, 20 Aug 2025 22:30:51 -0700 Subject: [PATCH 7/7] fix mp_map --- mapillary_tools/utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/mapillary_tools/utils.py b/mapillary_tools/utils.py index 36a48b057..0bf355127 100644 --- a/mapillary_tools/utils.py +++ b/mapillary_tools/utils.py @@ -231,9 +231,7 @@ def mp_map_maybe( initializer=configure_logger, initargs=(None, app_logger.getEffectiveLevel()), ) as executor: - futures = [executor.submit(func, item) for item in iterable] - for future in concurrent.futures.as_completed(futures): - yield future.result() + yield from executor.map(func, iterable) def configure_logger(