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

Remove broad Exception catch surrounding dnatracing #699

Closed
SylviaWhittle opened this issue Oct 15, 2023 · 3 comments
Closed

Remove broad Exception catch surrounding dnatracing #699

SylviaWhittle opened this issue Oct 15, 2023 · 3 comments
Labels
enhancement New feature or request v2.3.0

Comments

@SylviaWhittle
Copy link
Collaborator

Currently, we have a broad try except clause catching any and all Exceptions. This caused me to not notice dnatracing failing despite having a test for it, and allowing #696 to slip under our radars. (Clearly also the fault of me not writing a very good test! Succeeding even when the expected output was not complete). I think it is in our best interests to tighten this exception to only allow specific, known exceptions, which might prevent things like this going forward.

Thoughts @ns-rse ?

@SylviaWhittle SylviaWhittle added the enhancement New feature or request label Oct 15, 2023
@ns-rse
Copy link
Collaborator

ns-rse commented Oct 15, 2023

I was trying to include more try: ... except: ... in the refactoring of dnatracing. There is still a lot of code in there that needs refactoring though (e.g. pruning skeletons is pending as progress on incorporating iterative pruning routines up-stream to skan has not progressed as fast as we might have hoped due to annual leave and illness).

Removing the broad exception is definitely something that should be done though. It was put on because there was the desire for things to fail gracefully and errors on one image not to prevent all other images from being processed.

@ns-rse
Copy link
Collaborator

ns-rse commented Apr 3, 2024

Duplicate of #366 ?

@ns-rse
Copy link
Collaborator

ns-rse commented Nov 19, 2024

I think we can close this as the revision of logging reports when DNA tracing fails and suggests making a bug request.

See #976 which addressed #929.

@ns-rse ns-rse added the v2.3.0 label Nov 19, 2024
@ns-rse ns-rse closed this as completed Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2.3.0
Projects
None yet
Development

No branches or pull requests

2 participants