Skip to content

Commit 3338aa9

Browse files
committed
enh: minor adjustments of logic and cleanup of logging output
- invoke download in sequence, to allow for mixing different kinds of identifiers - revisit logging output types to reduce console clutter - change default logging level to info
1 parent 1fed366 commit 3338aa9

2 files changed

Lines changed: 35 additions & 27 deletions

File tree

idc_index/cli.py

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
# Set up logging for the CLI module
1616
logging.basicConfig(format="%(asctime)s - %(message)s", level=logging.DEBUG)
1717
logger_cli = logging.getLogger("cli")
18-
logger_cli.setLevel("WARNING")
18+
logger_cli.setLevel(logging.INFO)
1919

2020

2121
@click.group()
@@ -39,6 +39,7 @@ def set_log_level(log_level):
3939
logging_level = log_levels.get(log_level.lower(), logging.WARNING)
4040
logger_cli.debug(f"Setting the log level of index.py to {logging_level}")
4141
index.logger.setLevel(logging_level)
42+
logger_cli.setLevel(logging_level)
4243

4344

4445
@main.command()
@@ -129,6 +130,7 @@ def download_from_selection(
129130
set_log_level(log_level)
130131
# Create an instance of the IDCClient
131132
client = IDCClient()
133+
logger_cli.info(f"Downloading from IDC {client.get_idc_version()} index")
132134
# Parse the input parameters and pass them to IDCClient's download_from_selection method
133135
collection_id = (
134136
[cid.strip() for cid in (",".join(collection_id)).split(",")]
@@ -237,6 +239,7 @@ def download_from_manifest(
237239
set_log_level(log_level)
238240
# Create an instance of the IDCClient
239241
client = IDCClient()
242+
logger_cli.info(f"Downloading from IDC {client.get_idc_version()} index")
240243
logger_cli.debug("Inputs received from cli manifest download:")
241244
logger_cli.debug(f"manifest_file_path: {manifest_file}")
242245
logger_cli.debug(f"download_dir: {download_dir}")
@@ -264,7 +267,7 @@ def download_from_manifest(
264267
type=click.Choice(
265268
["debug", "info", "warning", "error", "critical"], case_sensitive=False
266269
),
267-
default="warning",
270+
default="info",
268271
help="Set the logging level for the CLI module.",
269272
)
270273
def download(generic_argument, log_level):
@@ -277,11 +280,13 @@ def download(generic_argument, log_level):
277280
# Create an instance of the IDCClient
278281
client = IDCClient()
279282

283+
logger_cli.info(f"Downloading from IDC {client.get_idc_version()} index")
284+
280285
download_dir = Path.cwd()
281286

282287
if Path(generic_argument).is_file():
283288
# Parse the input parameters and pass them to IDC
284-
logger_cli.debug("Detected manifest file, downloading from manifest.")
289+
logger_cli.info("Detected manifest file, downloading from manifest.")
285290
client.download_from_manifest(generic_argument, downloadDir=download_dir)
286291
# this is not a file manifest
287292
else:
@@ -300,28 +305,31 @@ def check_and_download(column_name, item_ids, download_dir, kwarg_name):
300305
return False
301306
unmatched_ids = list(set(item_ids) - set(matched_ids))
302307
if unmatched_ids:
303-
logger_cli.warning(
308+
logger_cli.debug(
304309
f"Partial match for {column_name}: matched {matched_ids}, unmatched {unmatched_ids}"
305310
)
306-
logger_cli.debug(f"Downloading from {column_name}")
311+
logger_cli.info(f"Identified matching {column_name}: {matched_ids}")
307312
client.download_from_selection(
308313
**{kwarg_name: matched_ids, "downloadDir": download_dir}
309314
)
310315
return True
311316

312-
# Check for matches in each column and download if matches found
313-
if not (
314-
check_and_download("collection_id", item_ids, download_dir, "collection_id")
315-
or check_and_download("PatientID", item_ids, download_dir, "patientId")
316-
or check_and_download(
317-
"StudyInstanceUID", item_ids, download_dir, "studyInstanceUID"
318-
)
319-
or check_and_download(
320-
"SeriesInstanceUID", item_ids, download_dir, "seriesInstanceUID"
321-
)
322-
):
317+
matches_found = 0
318+
matches_found += check_and_download(
319+
"collection_id", item_ids, download_dir, "collection_id"
320+
)
321+
matches_found += check_and_download(
322+
"PatientID", item_ids, download_dir, "patientId"
323+
)
324+
matches_found += check_and_download(
325+
"StudyInstanceUID", item_ids, download_dir, "studyInstanceUID"
326+
)
327+
matches_found += check_and_download(
328+
"SeriesInstanceUID", item_ids, download_dir, "seriesInstanceUID"
329+
)
330+
if not matches_found:
323331
logger_cli.error(
324-
"None of the values passed matched any of the four UUIDs: collection_id, PatientID, StudyInstanceUID, SeriesInstanceUID."
332+
"None of the values passed matched any of the identifiers: collection_id, PatientID, StudyInstanceUID, SeriesInstanceUID."
325333
)
326334

327335

idc_index/index.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -781,10 +781,10 @@ def _track_download_progress(
781781
show_progress_bar: bool = True,
782782
list_of_directories=None,
783783
):
784-
logger.info("Inputs received for tracking download:")
785-
logger.info(f"size_MB: {size_MB}")
786-
logger.info(f"downloadDir: {downloadDir}")
787-
logger.info(f"show_progress_bar: {show_progress_bar}")
784+
logger.debug("Inputs received for tracking download:")
785+
logger.debug(f"size_MB: {size_MB}")
786+
logger.debug(f"downloadDir: {downloadDir}")
787+
logger.debug(f"show_progress_bar: {show_progress_bar}")
788788

789789
runtime_errors = []
790790

@@ -798,7 +798,7 @@ def _track_download_progress(
798798

799799
logger.info("Initial size of the directory: %s bytes", initial_size_bytes)
800800
logger.info(
801-
"Approx. Size of the files need to be downloaded: %s bytes",
801+
"Approximate size of the files that need to be downloaded: %s bytes",
802802
total_size_bytes,
803803
)
804804

@@ -902,7 +902,7 @@ def _parse_s5cmd_sync_output_and_generate_synced_manifest(
902902
sync_size = merged_df["series_size_MB"].sum()
903903
sync_size_rounded = round(sync_size, 2)
904904

905-
logger.info(f"sync_size_rounded: {sync_size_rounded}")
905+
logger.debug(f"sync_size_rounded: {sync_size_rounded}")
906906

907907
if dirTemplate is not None:
908908
hierarchy = self._generate_sql_concat_for_building_directory(
@@ -1050,11 +1050,11 @@ def _s5cmd_run(
10501050
if sync_size < total_size:
10511051
logger.info(
10521052
"""
1053-
Destination folder is not empty and sync size is less than total size. Displaying a warning
1053+
Destination folder is not empty and sync size is less than total size.
10541054
"""
10551055
)
10561056
existing_data_size = round(total_size - sync_size, 2)
1057-
logger.warning(
1057+
logger.info(
10581058
f"Requested total download size is {total_size} MB, \
10591059
however at least {existing_data_size} MB is already present,\
10601060
so downloading only remaining upto {sync_size} MB\n\
@@ -1075,7 +1075,7 @@ def _s5cmd_run(
10751075
)
10761076
else:
10771077
logger.info(
1078-
"Not using s5cmd sync dry run as the destination folder is empty or sync dry or progress bar is not requested"
1078+
"Not using s5cmd sync as the destination folder is empty or sync or progress bar is not requested"
10791079
)
10801080
cmd = [
10811081
self.s5cmdPath,
@@ -1430,7 +1430,7 @@ def download_from_selection(
14301430
list_of_directories = result_df.path.to_list()
14311431
else:
14321432
list_of_directories = [downloadDir]
1433-
logger.info(
1433+
logger.debug(
14341434
"""
14351435
Temporary download manifest is generated and is passed to self._s5cmd_run
14361436
"""

0 commit comments

Comments
 (0)