Silent failures in straw/straw.py

[srcoulombe]

Is your feature request related to a problem? Please describe. Some functions in straw/straw.py print a message and return -1 when unexpected outcomes are encountered. Checking for a returned value of -1 could help, but 1) this isn't currently done in the code, 2) isn't ideal as it delegates the responsibility of catching these exceptions outside of the relevant function. Worse, it can let execution carry on for a while and then trigger an unexpected and hard-to-trace exception when the -1 value is subsequently used.

Describe the solution you'd like Explicitely raising typed exceptions with informative messages and documenting common cases for their occurrence.

Describe alternatives you've considered None, but suggestions are encouraged.

Example

    if (magic_string != b"HIC"):
        print('This does not appear to be a HiC file magic string is incorrect')
        return -1
    global version
    version = struct.unpack('<i',req.read(4))[0]
    if (version < 6):
        print("Version {0} no longer supported".format(str(version)))
        return -1

[srcoulombe]

I've got a branch that fixes these issues ready to compare. Some conflicts will be raised as it stands, so if the following could be answered, I could make the changes necessary to avoid the conflicts before merging:

1. The "norm" parameter to straw/straw.py is no longer being checked as per 80644d9. Should invalid norm values trigger an exception statement which would print the valid norm values?

2. Is there an incentive to maintain compatibility for python 3.<6? I'm just asking because if we can assume users will have python 3.6+, then f-strings would make some parts of the code much more legible.

[nchernia]

Sorry we dropped the ball on this and thanks for all your work on this repository.

For now I think it would be good to maintain compatibility with 3.<6, but we are willing to hear other opinions.

For invalid norms - people can add arbitrarily named norms to hic files so there's no such thing as an invalid norm anymore.

On Sat, Oct 26, 2019 at 9:07 PM srcoulombe notifications@github.com wrote:

I've got a branch that fixes these issues ready to compare. Some conflicts will be raised as it stands, so if the following could be answered, I could make the changes necessary to avoid the conflicts before merging:

1.

The "norm" parameter to straw/straw.py is no longer being checked as per 80644d9 https://github.com/srcoulombe/straw/commit/85045e5092a4858edddf027be1010a084e48921b. Should invalid norm values trigger an exception statement which would print the valid norm values? 2.

Is there an incentive to maintain compatibility for python 3.<6? I'm just asking because if we can assume users will have python 3.6+, then f-strings would make some parts of the code much more legible.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/aidenlab/straw/issues/42?email_source=notifications&email_token=AAK2EW3UTTNKPGZRIFSRBL3QQTSVDA5CNFSM4JFPO572YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECKUD7I#issuecomment-546652669, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAK2EW3ZX64QSRZAETGBCX3QQTSVDANCNFSM4JFPO57Q .

-- Neva Cherniavsky Durand, Ph.D. Pronouns: she, her, hers Assistant Professor, Aiden Lab www.aidenlab.org