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

Excepting/catching conversion errors instead of just logging them #113

Open
mikbuch opened this issue May 13, 2022 · 2 comments
Open

Excepting/catching conversion errors instead of just logging them #113

mikbuch opened this issue May 13, 2022 · 2 comments

Comments

@mikbuch
Copy link

mikbuch commented May 13, 2022

Hi,

I'd like to see an option (e.g., an argument) to throw an exception, instead of just logging it as a warning (see https://github.com/icometrix/dicom2nifti/blob/main/dicom2nifti/convert_dir.py#L89-L91).

I guess that thought behind the current version was to just skip the directories in which there are some problems, but from the programmatic point of view I'd like to be able to catch/except errors myself.

My suggestion is to add an argument throw_exceptions with the default value of False (for backward compatibility):

def convert_directory(dicom_directory, output_folder, compression=True, reorient=True, throw_exceptions=False):

https://github.com/icometrix/dicom2nifti/blob/main/dicom2nifti/convert_dir.py#L26

and then, when excepting an error:

        except:  
            if throw_exceptions:
                # Rising an exception is not a default behavior. By default, e.g., conversion errors are just logged as warnings.
                raise
            else:
                # Explicitly capturing app exceptions here to be able to continue processing.
                logger.info("Unable to convert: %s" % base_filename) 
                traceback.print_exc()

https://github.com/icometrix/dicom2nifti/blob/main/dicom2nifti/convert_dir.py#L89-L91

Then, if someone would like to catch errors themselves (e.g., to count how many errors of particular kind there are) throw_exceptions would be set to True.

What do you think? I can create a PR, if you like the idea.

CC: @japarty

@mikbuch
Copy link
Author

mikbuch commented May 25, 2022

Hi, we found a workaround to handle/catch error information from the logger:

import logging
import dicom2nifti
from dicom2nifti import convert_directory

class MyLogger(logging.Handler):
    def handle(*args):
        for item in args:
            if 'Unable to convert' in str(item):
                print("Catched!")
                raise

my_logger = MyLogger(logging.INFO)  # set out handler to catch at INFO level
logger = dicom2nifti.convert_dir.logger
logger.addHandler(my_logger)
logger.setLevel(logging.INFO)  # set main logger level to INFO
try:
    convert_directory(...)
except:
    do_something()

Credits to: piowyr, thanks!

CC: japarty

@mikbuch mikbuch closed this as completed May 25, 2022
@mikbuch
Copy link
Author

mikbuch commented May 25, 2022

I miss-clicked and closed the issue.

The issue is still relevant. We only found a workaround.

@mikbuch mikbuch reopened this May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant