Skip to content

schemaview.py: deprecate permissible_value_parent, replace with permissible_value_parents #479

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 22, 2025
Merged

schemaview.py: deprecate permissible_value_parent, replace with permissible_value_parents #479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b908991
Deprecate `permissible_value_parent` and replace it with `permissible…
ialarmedalien Oct 21, 2025
d0e9d93
Fix docs for permissible_value_parents/children
ialarmedalien Oct 21, 2025
82624a2
Merge branch 'main' into schemaview_fix_permissible_value_parent_plur…
ialarmedalien Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions linkml_runtime/utils/schemaview.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,9 @@ def _parents(self, e: Element, imports: bool = True, mixins: bool = True, is_a:
def class_parents(
self, class_name: CLASS_NAME, imports: bool = True, mixins: bool = True, is_a: bool = True
) -> list[ClassDefinitionName]:
""":param class_name: child class name
"""Get the parents of a class.

:param class_name: child class name
:param imports: include import closure
:param mixins: include mixins (default is True)
:return: all direct parent class names (is_a and mixins)
Expand Down Expand Up @@ -1167,7 +1169,9 @@ def type_roots(self, imports: bool = True) -> list[TypeDefinitionName]:
def enum_parents(
self, enum_name: ENUM_NAME, imports: bool = False, mixins: bool = False, is_a: bool = True
) -> list[EnumDefinitionName]:
""":param enum_name: child enum name
"""Get the parents of an enum.

:param enum_name: child enum name
:param imports: include import closure (False)
:param mixins: include mixins (default is False)
:return: all direct parent enum names (is_a and mixins)
Expand Down Expand Up @@ -1204,12 +1208,20 @@ def enum_ancestors(
**kwargs,
)

@lru_cache(None)
@deprecated("Use `permissible_value_parents` instead")
Copy link
Contributor

@Silvanoc Silvanoc Oct 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@ialarmedalien ialarmedalien Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. 🤷

Copy link
Contributor

@Silvanoc Silvanoc Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

@ialarmedalien ialarmedalien Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@ialarmedalien ialarmedalien Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an issue about the @deprecated decorator:

linkml/linkml#2969

Copy link
Contributor

@Silvanoc Silvanoc Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

def permissible_value_parent(
self, permissible_value: str, enum_name: ENUM_NAME
) -> list[str | PermissibleValueText]:
""":param enum_name: child enum name
return self.permissible_value_parents(permissible_value, enum_name)

@lru_cache(None)
def permissible_value_parents(
self, permissible_value: str, enum_name: ENUM_NAME
) -> list[str | PermissibleValueText]:
"""Get the parents of a permissible value.

:param permissible_value: permissible value
:param enum_name: enum for which this is a permissible value
:return: all direct parent enum names (is_a)
"""
enum = self.get_enum(enum_name, strict=True)
Expand All @@ -1223,8 +1235,10 @@ def permissible_value_parent(
def permissible_value_children(
self, permissible_value: str, enum_name: ENUM_NAME
) -> list[str | PermissibleValueText]:
""":param enum_name: parent enum name
"""Get the children of a permissible value.

:param permissible_value: permissible value
:param enum_name: enum for which this is a permissible value
:return: all direct child permissible values (is_a)
"""
enum = self.get_enum(enum_name, strict=True)
Expand Down Expand Up @@ -1262,7 +1276,7 @@ def permissible_value_ancestors(
:rtype: list[str]
"""
return _closure(
lambda x: self.permissible_value_parent(x, enum_name),
lambda x: self.permissible_value_parents(x, enum_name),
permissible_value_text,
reflexive=reflexive,
depth_first=depth_first,
Expand Down
13 changes: 7 additions & 6 deletions tests/test_utils/test_schemaview.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ def test_metamodel_in_schemaview() -> None:
for tn in ["uriorcurie", "string", "float"]:
assert tn in view.all_types()
assert tn not in view.all_types(imports=False)
for cn, c in view.all_classes().items():
for cn in view.all_classes():
uri = view.get_uri(cn, expand=True)
assert uri is not None
if cn not in ["structured_alias", "UnitOfMeasure", "ValidationReport", "ValidationResult"]:
Expand Down Expand Up @@ -1598,7 +1598,7 @@ def test_type_roots(schema: str, type_roots: set[str]) -> None:


def test_all_enums(schema_view_with_imports: SchemaView) -> None:
"""Test all_enums"""
"""Test all_enums."""
view = schema_view_with_imports

for en, e in view.all_enums().items():
Expand Down Expand Up @@ -1809,6 +1809,7 @@ def gen_schema_name(range_tuple: tuple[str, str | None, str | None]) -> str | No

def gen_range_file_with_default(range_id: str, tmp_path_factory: pytest.TempPathFactory) -> Path:
"""Generate a copy of the range file with a default_range added and return the path.

Obviates the need for maintaining a copy with a default_range tagged to the end.

:param range_id: the range file to use; must be one of RL, RI
Expand Down Expand Up @@ -2232,28 +2233,28 @@ def test_permissible_value_relationships(schema_view_no_imports: SchemaView) ->
pv_cat = animal_enum.permissible_values["CAT"]
assert pv_cat.text == "CAT"
assert pv_cat.is_a is None
assert view.permissible_value_parent("CAT", animals) == []
assert view.permissible_value_parents("CAT", animals) == []
assert view.permissible_value_ancestors("CAT", animals) == ["CAT"]
assert set(view.permissible_value_children("CAT", animals)) == {"LION", "TABBY"}
assert set(view.permissible_value_descendants("CAT", animals)) == {"CAT", "LION", "ANGRY_LION", "TABBY"}

pv_tabby = animal_enum.permissible_values["TABBY"]
assert pv_tabby.is_a == "CAT"
assert view.permissible_value_parent("TABBY", animals) == ["CAT"]
assert view.permissible_value_parents("TABBY", animals) == ["CAT"]
assert view.permissible_value_ancestors("TABBY", animals) == ["TABBY", "CAT"]
assert view.permissible_value_children("TABBY", animals) == []
assert view.permissible_value_descendants("TABBY", animals) == ["TABBY"]

pv_lion = animal_enum.permissible_values["LION"]
assert pv_lion.is_a == "CAT"
assert view.permissible_value_parent("LION", animals) == ["CAT"]
assert view.permissible_value_parents("LION", animals) == ["CAT"]
assert view.permissible_value_ancestors("LION", animals) == ["LION", "CAT"]
assert view.permissible_value_children("LION", animals) == ["ANGRY_LION"]
assert view.permissible_value_descendants("LION", animals) == ["LION", "ANGRY_LION"]

pv_angry_lion = animal_enum.permissible_values["ANGRY_LION"]
assert pv_angry_lion.is_a == "LION"
assert view.permissible_value_parent("ANGRY_LION", animals) == ["LION"]
assert view.permissible_value_parents("ANGRY_LION", animals) == ["LION"]
assert view.permissible_value_ancestors("ANGRY_LION", animals) == ["ANGRY_LION", "LION", "CAT"]
assert view.permissible_value_children("ANGRY_LION", animals) == []
assert view.permissible_value_descendants("ANGRY_LION", animals) == ["ANGRY_LION"]
Expand Down
Loading