movement icon indicating copy to clipboard operation
movement copied to clipboard

Drop the `.move` accessor

Open niksirbi opened this issue 1 year ago • 0 comments

The problem

We currently have two ways of applying kinematics and filtering operations:

  1. The "functional" way: velocity = compute_velocity(ds.position)
  2. The "accessor" way: velocity = ds.move.compute_velocity()

For more information on the accessor approach, refer to the relevant section of our documentation.

This dual approach leads to several issues:

  • There are two methods to achieve the same result, both of which need to be tested, documented, and maintained.
  • It’s difficult to explain the accessor method (i.e., the .move keyword) to new users, especially those unfamiliar with object-oriented programming.
  • New contributors may find it challenging to understand what needs to be modified when adding a new filter or kinematic variable.

Proposed solution

Completely remove the accessor:

  • Delete the corresponding module and tests.
  • Remove all mentions of the accessor from the documentation.
  • Update all examples to use the functional syntax.

Alternatives

  • Maintain the status quo: This avoids a breaking change, but it might continue to confuse users and increase maintenance costs.
  • Keep the accessor but do not document it: This is the worst of both worlds, as it retains the maintenance burden without providing value to users.

Additional considerations

Things we will "lose" if we remove the accessor:

  • The ability to apply the same filter to multiple data variables with a single method call (using the data_vars argument).
  • The validate method, which we were considering removing anyway (#257).

During our last meeting, we agreed that these benefits do not justify the added maintenance burden.

I'll leave this open for ~ 1 week, if no one objects, we will start the removal process.

niksirbi avatar Oct 08 '24 10:10 niksirbi