PyLops / pylops

PyLops – A Linear-Operator Library for Python
https://pylops.readthedocs.io
GNU Lesser General Public License v3.0
430 stars 102 forks source link

incorrect check on dips argument in in `slope_estimation #572

Closed Beramos closed 8 months ago

Beramos commented 9 months ago

The documentation states the following:

image

However, the logic flow in the code is the opposite:

https://github.com/PyLops/pylops/blob/0d19f5689978cb71e743bf673d4ce775e22817c6/pylops/utils/signalprocessing.py#L249-L253

I double checked with the original implementation of dips_estimate

https://github.com/PyLops/pylops/blob/c5844c4a4b25cdb74a5cef12b926d6d91da20095/pylops/utils/signalprocessing.py#L79-L182

And it is inline with the current documentation. It appears that the bug was introduced when dips_estimate and slope_estimate` merged.

I have a PR ready for this.

mrava87 commented 8 months ago

Thanks @Beramos for noticing this.

@cako, since you did quite some changes to this part of the code, do you agree with this? If so, I can go ahead and merge the PR

mrava87 commented 8 months ago

@cako?