Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anti pattern: properties that depend on independent properties #359

Open
adku1173 opened this issue Nov 18, 2024 · 0 comments · May be fixed by #374
Open

Anti pattern: properties that depend on independent properties #359

adku1173 opened this issue Nov 18, 2024 · 0 comments · May be fixed by #374
Assignees
Labels

Comments

@adku1173
Copy link
Member

We should write a test to avoid the construct of properties that rely on other properties that do not depend on any other trait (mainly when calculating the digest!). The following example does not trigger any notification (same digest, although property value changes):

from acoular.internal import digest
from traits.api import Property, HasPrivateTraits, Int, cached_property 


class Test( HasPrivateTraits ):

    some_property = Property()

    _some_property = Int

    digest = Property( depends_on = ['some_property'] )

    def _get_some_property( self ):
        return self._some_property

    def _set_some_property( self, value ):
        self._some_property = value

    @cached_property
    def _get_digest(self):
        return digest(self)

if __name__ == "__main__":

    t = Test()
    t.some_property = 1
    print(t.digest)
    t.some_property = 2
    print(t.digest)

results in

_9d4ea2a37415bda19695e6210cf513a0 _9d4ea2a37415bda19695e6210cf513a0

Fortunately, such constructs hardly exist, but we should ensure they don't exist. Found it in

  • LineGrid (size trait)
  • MergeGrid (grid_digest trait)
  • GenericSignalGenerator (numsamples trait)
  • TimeSamples (_datachecksum trait)
  • PowerSpectra (calib trait)
  • SpatialInterpolator (mics_virtual trait)

which indeed creates problems:

import acoular as ac
import numpy as np

mics_virtual = ac.MicGeom( mpos_tot = np.random.randn(3,3) )
print('mic digest', mics_virtual.digest)
interp = ac.SpatialInterpolator()
print('SpatialInterpolator digest', interp.digest)

# change the mics_virtual
mics_virtual.mpos_tot = np.random.randn(3,3)
print('mic digest', mics_virtual.digest)
print('SpatialInterpolator digest', interp.digest)

Output:

mic digest _996f9dfb5a9ea06ee04dab3144162104
SpatialInterpolator digest _383696c4c684315958bc46508cd29e8a
mic digest _40bd94e59ee802db8c5a22ca92d56fba
SpatialInterpolator digest _383696c4c684315958bc46508cd29e8a
@adku1173 adku1173 added the bug label Nov 18, 2024
@adku1173 adku1173 added this to the Release v25.01 milestone Nov 21, 2024
@artpelling artpelling linked a pull request Nov 28, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants