christiansandberg / canopen

CANopen for Python
http://canopen.readthedocs.io/
MIT License
431 stars 195 forks source link

Give deprecation warning on old interfaces #479

Open sveinse opened 1 month ago

sveinse commented 1 month ago

We should probably implement deprecation warnings in the code. We have changed a few class names recently and we have code in place for compatibility. E.g. canopen.sdo.base defines Record as alias to SdoRecord and so on. I agree that we must not introduce breaking change and have aliases like this, but I think we should inform them as deprecated so we steer the users towards the new API.

We should also write a statement when these aliases will stop working.

acolomb commented 1 month ago

Frankly I don't see this as high priority. Sure if you want to add it, go ahead but keep it short. Using these class names outside the library is unusual and I think the documentation doesn't even mention them in any guide or example. So the usage will be very low and I expect anyone depending somewhat on the library internals to do proper testing or at least read the change log thoroughly before upgrading a dependency.

As for the compatibility aliases, they don't bother me at all where they currently live. So there's no planned time frame to remove them.

sveinse commented 1 month ago

Any (active developed) API will evolve over time. It is wise to have a strategy of how to introduce new methods and classes, how to deprecate and then finally remove obsolete ones:

  1. When do we inform the user of deprecation?
  2. When do we remove the deprecated methods and classes?
acolomb commented 1 month ago

I guess each of these bullet points need someone to put in the work. If you're up to it, make some suggestions. So far we simply don't have a "grand plan" about deprecation, so I'd handle it lazily until it gets annoying somewhere. I love software that just keeps working and doesn't break other code arbitrarily. Keeping the old names is the simplest approach to that as it requires no effort really.