schemaview.py: deprecate permissible_value_parent, replace with permissible_value_parents

[ialarmedalien]

Note: base branch is #478; suggest reviewing/merging that branch first.

Fixing the name of the permissible_value_parent function to match the other *_parents functions, which are plural.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.20%. Comparing base (4c4bbc9) to head (82624a2).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #479      +/-   ##
==========================================
+ Coverage   78.19%   78.20%   +0.01%     
==========================================
  Files          52       52              
  Lines        4522     4525       +3     
  Branches      985      985              
==========================================
+ Hits         3536     3539       +3     
  Misses        768      768              
  Partials      218      218              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Silvanoc]

This PR depends on #478, which depends on #477, which depends on #476,... I think you're gonna need a graph to document all these dependencies 🙃

[Silvanoc]

You are changing a public interface here.

Is this decoration enough for downstream projects? Or are they expecting deprecations to follow the LinkML guidelines?

BTW funny enough in the SchemaView public documentation you can find functions (like permissible_value_ancestors) reporting closure of permissible_value_parents, confirming the need of this change.

[ialarmedalien]

I was debating this point for a while (especially since deprecation was top-of-mind from looking at your PR). In the end, I followed the approach that was used with other functions in the SchemaView class as the only references that I could find to the method were in the tests. 🤷

[Silvanoc]

Being a public interface, some projects might have used it and you won't notice it.

In the end I'm not sure if downstream projects are already taking into consideration the current deprecation guidelines or not.

Why aren't you following the guidelines?

[ialarmedalien]

Strictly speaking, it isn't changing a public interface because the existing function redirects to a second function that does exactly the same thing. The existing function will not now stop working or return a different result. Downstream consumers can continue to use permissible_value_parent without any negative side effects.

I used this deprecation method partially because the public API has been preserved, and partially because there are existing functions in schemaview.py that have "deprecated" aliases that are set up in the same way, e.g.

    @deprecated("Use `all_slots` instead")
    @lru_cache(None)
    def all_slot(self, **kwargs: dict[str, Any]) -> dict[SlotDefinitionName, SlotDefinition]:
        """Retrieve all slots from the schema.

        :param imports: include imports closure
        :return: all slots in schema view
        """
        return self.all_slots(**kwargs)

    @lru_cache(None)
    def all_slots(
        self, ordered_by: OrderedBy = OrderedBy.PRESERVE, imports: bool = True, attributes: bool = True
    ) -> dict[SlotDefinitionName, SlotDefinition]:
        """Retrieve all slots from the schema."""

The same applies to the functions all_* for * in class, enum, type, prefix, subset, and element.

[ialarmedalien]

Added an issue about the @deprecated decorator:

linkml/linkml#2969

[Silvanoc]

Strictly speaking, it isn't changing a public interface because the existing function redirects to a second function that does exactly the same thing. The existing function will not now stop working or return a different result. Downstream consumers can continue to use permissible_value_parent without any negative side effects.

You are right on that. But if someone else in the future comes to the idea of cleaning up all deprecations, then it will happen. Therefore I think that with the issue that you've created it should be fine.

[Silvanoc]

LGTM only clarification WRT the use of the @deprecated decorator needed.

[amc-corey-cox]

This PR is now only dependent on #478 which has had it's dependencies satisfied. We'll get these both rolled in once @ialarmedalien has had a chance to respond on that one and respond to the @deprecated decorator comment above.

[Silvanoc]

All concerns addressed

[ialarmedalien]

Thanks for the quick turnaround on these, @Silvanoc and @amc-corey-cox !