Changed default value of ignore_diags to auto instead of None in coverage

[efriman]

This is a simple suggestion of a name change for the default value of ignore_diags in coverage. I had assumed None was equivalent to 0, so I propose to rename the default to "auto"

[Phlya]

Looks good to me! Thank you @efriman

[gfudenberg]

I agree that None as default could be confusing. Perhaps all tools that take this argument should be updated simultaneously, though? Also, I can't remember if this has been discussed and set as None for consistency with cooler? Maybe @nvictus @sergpolly recall?

[sergpolly]

not sure if this was for consistency with cooler ...
but if we want to go ahead with this - i think this needs to be propagated throughout cooltools ! not a function at a time ...

imho would be nice to hide this cooler-hack clr._load_attrs(clr.root.rstrip("/") + "/bins/weight")["ignore_diags"] into a tiny function:

def _get_cooler_ignore_diags(clr, clr_weight_name="weight"):
    """
    extracts number of diagonals that were ignored during balancing of the clr
    """
    clr_weight_uri = clr.root.rstrip("/") + f"/bins/{clr_weight_name}"
    return clr._load_attrs(clr_weight_uri)["ignore_diags"]

• I'm not 100% sure if the URI depend on the weight colyumn name, like I drafted it there ...
• Also, load_attr could be potentially safeguarded with a try catch situation to yield a human readable error

[efriman]

Yes that makes sense!

I think the main thing for me is that ignore_diags behaviour should be consistent across packages and functions. My intuitive sense would be that None or False or 0 would all lead to not ignoring diagonals. But None could also be that it takes it from the ignore_diags in the cooler attribute. If people agree then I could try to make it like that.

[sergpolly]

there is nothing special between ignore_diags and None in the cooler itself (source: quick text search for "ignore_diags" and "ignore-diags") - it is either False or int there and seems to be applicable only for balancing

So from that perspective changing default ignore_diags to "auto" should not introduce any inter-package inconsistencies