ContactEngineering / SurfaceTopography

Read and analyze surface topographies
https://contactengineering.github.io/SurfaceTopography/
MIT License
15 stars 9 forks source link

Bug/translated topography #346

Closed sannant closed 7 months ago

sannant commented 8 months ago
def test_translate_setter():
    topography = Topography(np.array([[0, 1, 0], [0, 0, 0]]),
                            physical_sizes=(4., 3.))

    translated_t = topography.translate()

    translated_t.offset = (1, 0)
    assert (translated_t.heights()
            ==
            np.array([[0, 0, 0],
                      [0, 1, 0]])).all()
    topography = Topography(np.array([[0, 1, 0], [0, 0, 0]]),
                            physical_sizes=(4., 3.), periodic=True)
    assert topography.translate(offset=(1, 0)).is_periodic

this previously existing test implies that it is ok to translate nonperiodic topographies (the first part, since by default, topographies are nonperiodic).

I think that this is bad behavior because the translation is implemented via a numpy.roll.

Do we agree to raise an error when a nonperiodic topography is translated ?

sannant commented 7 months ago

@pastewka I need your opinion on the comment above

pastewka commented 7 months ago

Agree to raise the error. Can you indicate in the changelog that this is an API change (prefix with API, not changelog) which triggers a new major version release.

sannant commented 7 months ago

Ok