eqcorrscan / EQcorrscan

Earthquake detection and analysis in Python.
https://eqcorrscan.readthedocs.io/en/latest/
Other
166 stars 86 forks source link

Party.min_chans should include the "min_chan" value #369

Closed ThomasLecocq closed 4 years ago

ThomasLecocq commented 4 years ago

I'm trying to reduce a Party object to remove detections with less than 2 channels.

My understanding of the documentation:

Remove detections with fewer channels used than min_chans
        :param min_chans: Minimum number of channels to allow a detection.

is that if I pass 2, I should have a Party with any detection that has 2, 3, ... detections present.

BUT, the code says:

        declustered = Party()
        for family in self.families:
            fam = Family(family.template)
            for d in family.detections:
                if d.no_chans > min_chans:
                    fam.detections.append(d)
            declustered.families.append(fam)
        self.families = declustered.families
        return self

therefore using a strict "larger than" min_chans.

From the doc I woudl expect https://github.com/eqcorrscan/EQcorrscan/blob/master/eqcorrscan/core/match_filter/party.py#L973 to be if d.no_chans >= min_chans: ?

Cheers, Tom

calum-chamberlain commented 4 years ago

That does seem like the correct interpretation of the docs! Feel free to make that change @ThomasLecocq - do you want me to give you access to the EQcorrscan repository directly, or would you rather work on a fork?

ThomasLecocq commented 4 years ago

@calum-chamberlain if you're ok with it, I'm happy to access the repo directly, and I'll work in a branch (off master?) and propose a PR you (only) can accept ?

I'll also write some doc + tutorials later :-)

calum-chamberlain commented 4 years ago

Ace, thanks. I added you as a collaborator on EQcorrscan, which should give you write access to the repo.

Docs help would be sweet!

calum-chamberlain commented 4 years ago

I added some temporary docs to clarify the wrong use of min_chans in develop, so I'm going to close this for now.

ThomasLecocq commented 4 years ago

wooops, I did forget to do this PR , did I ? :s

calum-chamberlain commented 4 years ago

All good! I just did a very quick change, any other help/more useful docs remain helpful!