can be easily replaced [...]

Easily? Yes.

-$item->getAllAliases();
+$item->getFingerprint()->getAliasGroups()->toTextArray();

Nice, readable and maintainable? No.

Deprecated methods like these should be phased out some day, but I disagree that this should be done in the planed DataModel 3.0. DataModel 3.0 should, in my opinion, focus on phasing out Claim, and avoid Fingerprint related stuff until the discussion is finished.

T75496 does not depend on the removal of methods. This just doesn't make sense. Registries and strategies that allow new entity types can easily be introduced without phasing out stuff in Entity first.

So please remove both blockers.

Relevant patches, both blocked because of my first argument:

• https://github.com/wmde/WikibaseDataModel/pull/475
• https://github.com/wmde/WikibaseDataModel/pull/476

Introduction of new code is not blocked on deprecated code.

So if you think in the long terms a bit you will understand [...]

I'm not going to continue a discussion on this level.

Can we please not create an issue per PR and in doing so duplicate the discussions? We are also not creating an issue per commit on Gerrit.

Most discussion happened here: https://github.com/wmde/WikibaseDataModel/pull/476

Had a chat with @Bene. This is what's left in Entity and not deprecated:

• Entity::getFingerprint already has an interface, FingerprintProvider. In 3.1, the method should be deprecated in Entity and un-deprecated in Item and Property (via duplicate implementations, similar to Item::setLabel and such).
• Entity::setFingerprint needs investigation. The most flexible solution is to create a FingerprintHolder with a getter and a setter, similar to StatementListHolder, make Item and Property implement this interface and deprecate the methods in Entity. But do we really need setFingerprint? If getFingerprint is guaranteed to not return a copy, all manipulations can be done on the returned Fingerprint object and setFingerprint never needs to be called. What we need to do:
  • Check if all usages of setFingerprint (especially the ones in Deserializers) are optional and can be removed.
  • The guarantee that getFingerprint does not return a copy must be added to the methods documentation.
• Entity::isEmpty should be deprecated and un-deprecated in Item and Property.
• Entity::clear should be deprecated and later die. Just create a new Item if you need a new item. Repurposing existing entities is ugly.
• Entity::copy. Not sure about this. It used a lot, more than 100 times, mostly in tests obviously but also in production. But it's very, very trivial and should be avoided anyway. Better create a new object.

In T98290#1424169, @thiemowmde wrote:

Had a chat with @Bene. This is what's left in Entity and not deprecated:

• Check if all usages of setFingerprint (especially the ones in Deserializers) are optional and can be removed.
• The guarantee that getFingerprint does not return a copy must be added to the methods documentation.

Just don't use Fingerprint at all. However, the question whether to return a clone from the getter exists for all mutable classes. This will be the same when dealing with TermList objects.

• Entity::isEmpty should be deprecated and un-deprecated in Item and Property.

This should probably go into EntityDocument if it is needed. A generic method to check if an entity is empty seems sensible. It's also needed by EntityContent.

• Entity::clear should be deprecated and later die. Just create a new Item if you need a new item. Repurposing existing entities is ugly.

Agreed

• Entity::copy. Not sure about this. It used a lot, more than 100 times, mostly in tests obviously but also in production. But it's very, very trivial and should be avoided anyway. Better create a new object.

The implementation is nasty, but we do need a generic copying mechanism. Since EntityContent is mutable, it needs a deep clone mechanism. Creating a new object with the same content is exactly what is done here, the question is jut - how?

https://github.com/wmde/WikibaseDataModel/pull/515

Change 222978 had a related patch set uploaded (by Bene):
Remove usage of Entity::copy

https://gerrit.wikimedia.org/r/222978