brmather / pycurious

Python package for computing the Curie depth from the magnetic anomaly
https://brmather.github.io/pycurious/
GNU Lesser General Public License v3.0
38 stars 17 forks source link

[REVIEW] Check equal node spacing #14

Closed santisoler closed 5 years ago

santisoler commented 5 years ago

Part of https://github.com/openjournals/joss-reviews/issues/1544.

When checking if the node spacing is equal on both northing and easting directions, a maximum difference of 1 meter has been hardcoded: https://github.com/brmather/pycurious/blob/759fc520c22503f0b40b6a63faa4e632dd9f1e88/pycurious/grid.py#L74-L75

Would be nice to use np.allclose(dx, dy) instead of hardcoding a minumum difference that could raise some unwanted results. For example, if you have a very dense survey with one measurement every 10m and dx is around 1m different from dy, you'll finally get a significant error on the wavenumbers and therefore on the estimated depths even that error won't be reflected on the statistical errors for these depths.

Do you agree?

Besides, wouldn't be better to raise an error instead of a warning? Using non equal spaced grids is acceptable for further computations?

brmather commented 5 years ago

Yes this makes sense. I haven't encountered such an issue, but that's not to say it won't happen to someone else. This has been addressed in 585fd432e1ad517b09989b87d461e59693cd5320 thanks!