Repository: linkml/linkml

Issue:
edge case: relmodel_transformer may inject primary keys with clashing labels
by default when relmodel_transformer creates linking tables, it needs a primary key on the linked table. If a slot with an identifier is present, it is used. If it is not present, then column with name `id` is injected. This creates a clash if the class already has a non-identifier field called `id`.

Discussion:
codecov-commenter: # [Codecov](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml) Report
> Merging [#1256](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml) (6ed354d) into [main](https://codecov.io/gh/linkml/linkml/commit/759802c15abb90911f94f6c291a3d2f2f82ac3f4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml) (759802c) will **increase** coverage by `0.12%`.
> The diff coverage is `88.26%`.

```diff
@@            Coverage Diff             @@
##             main    #1256      +/-   ##
==========================================
+ Coverage   80.02%   80.15%   +0.12%     
==========================================
  Files          75       76       +1     
  Lines        8610     8852     +242     
  Branches     2121     2149      +28     
==========================================
+ Hits         6890     7095     +205     
- Misses       1312     1337      +25     
- Partials      408      420      +12     
```


| [Impacted Files](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml) | Coverage Δ | |
|---|---|---|
| [...tors/sqlalchemy/sqlalchemy\_declarative\_template.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL2dlbmVyYXRvcnMvc3FsYWxjaGVteS9zcWxhbGNoZW15X2RlY2xhcmF0aXZlX3RlbXBsYXRlLnB5) | `100.00% <ø> (ø)` | |
| [linkml/generators/pythongen.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL2dlbmVyYXRvcnMvcHl0aG9uZ2VuLnB5) | `88.42% <50.00%> (-1.05%)` | :arrow_down: |
| [linkml/utils/sqlutils.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL3V0aWxzL3NxbHV0aWxzLnB5) | `56.27% <55.55%> (-0.16%)` | :arrow_down: |
| [linkml/transformers/relmodel\_transformer.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL3RyYW5zZm9ybWVycy9yZWxtb2RlbF90cmFuc2Zvcm1lci5weQ==) | `94.90% <80.00%> (-1.73%)` | :arrow_down: |
| [linkml/utils/schema\_builder.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL3V0aWxzL3NjaGVtYV9idWlsZGVyLnB5) | `84.15% <85.00%> (-5.18%)` | :arrow_down: |
| [linkml/generators/docgen.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL2dlbmVyYXRvcnMvZG9jZ2VuLnB5) | `85.20% <85.36%> (-1.15%)` | :arrow_down: |
| [linkml/workspaces/example\_runner.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL3dvcmtzcGFjZXMvZXhhbXBsZV9ydW5uZXIucHk=) | `80.00% <85.52%> (+4.00%)` | :arrow_up: |
| [linkml/generators/erdiagramgen.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL2dlbmVyYXRvcnMvZXJkaWFncmFtZ2VuLnB5) | `97.35% <100.00%> (ø)` | |
| [linkml/generators/linkmlgen.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL2dlbmVyYXRvcnMvbGlua21sZ2VuLnB5) | `81.35% <100.00%> (ø)` | |
| [linkml/generators/pydanticgen.py](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sL2dlbmVyYXRvcnMvcHlkYW50aWNnZW4ucHk=) | `83.78% <100.00%> (+2.16%)` | :arrow_up: |
| ... and [20 more](https://codecov.io/gh/linkml/linkml/pull/1256?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml)


Patch:
diff --git a/linkml/generators/common/__init__.py b/linkml/generators/common/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/linkml/generators/common/type_designators.py b/linkml/generators/common/type_designators.py
new file mode 100644
index 00000000..adea0af2
--- /dev/null
+++ b/linkml/generators/common/type_designators.py
@@ -0,0 +1,17 @@
+from linkml_runtime.linkml_model.meta import SlotDefinition, ClassDefinition
+from linkml_runtime.utils.schemaview import SchemaView
+from linkml_runtime.utils.formatutils import camelcase
+
+def get_type_designator_value(sv: SchemaView, type_designator_slot: SlotDefinition, class_def: ClassDefinition) -> str:
+    """
+        retuns the correct value for a type designator field for a given class, depending on its range
+        this implements the logic described in https://github.com/linkml/linkml/issues/945:
+    """
+    target_value = None
+    if type_designator_slot.range == 'uri':
+        target_value = sv.get_uri(class_def,expand=True,native=True)
+    elif type_designator_slot.range == 'string':
+        target_value = camelcase(class_def.name)
+    else:
+        target_value = sv.get_uri(class_def,expand=False)
+    return target_value
diff --git a/linkml/generators/pydanticgen.py b/linkml/generators/pydanticgen.py
index f091b720..1b32f23b 100644
--- a/linkml/generators/pydanticgen.py
+++ b/linkml/generators/pydanticgen.py
@@ -14,6 +14,7 @@ from linkml_runtime.linkml_model.meta import (Annotation, ClassDefinition,
 from linkml_runtime.utils.formatutils import camelcase, underscore
 from linkml_runtime.utils.schemaview import SchemaView
 
+from linkml.generators.common.type_designators import get_type_designator_value
 from linkml._version import __version__
 from linkml.generators.oocodegen import OOCodeGenerator
 from linkml.utils.generator import shared_arguments
@@ -26,7 +27,7 @@ default_template = """
 from __future__ import annotations
 from datetime import datetime, date
 from enum import Enum
-from typing import List, Dict, Optional, Any
+from typing import List, Dict, Optional, Any, Union
 from pydantic import BaseModel as BaseModel, Field
 from linkml_runtime.linkml_model import Decimal
 
@@ -200,15 +201,17 @@ class PydanticGenerator(OOCodeGenerator):
             for slot_name in sv.class_slots(class_def.name):
                 slot = sv.induced_slot(slot_name, class_def.name)
                 if slot.designates_type:
+                    target_value = get_type_designator_value(sv, slot, class_def)
                     slot_values[camelcase(class_def.name)][
                         slot.name
-                    ] = f'"{default_prefix}:{camelcase(class_def.name)}"'
+                    ] = f'"{target_value}"'
                     if slot.multivalued:
                         slot_values[camelcase(class_def.name)][slot.name] = (
                             "["
                             + slot_values[camelcase(class_def.name)][slot.name]
                             + "]"
                         )
+                    slot_values[camelcase(class_def.name)][slot.name] = slot_values[camelcase(class_def.name)][slot.name] + ", const=True"
                 # Multivalued slots that are either not inlined (just an identifier) or are
                 # inlined as lists should get default_factory list, if they're inlined but
                 # not as a list, that means a dictionary
@@ -268,11 +271,14 @@ class PydanticGenerator(OOCodeGenerator):
 
         # Inline the class itself only if the class is defined as inline, or if the class has no
         # identifier slot and also isn't a mixin.
-        if slot.inlined or (
+        if slot.inlined or slot.inlined_as_list or (
             sv.get_identifier_slot(range_cls.name) is None
             and not sv.is_mixin(range_cls.name)
         ):
-            return f"{camelcase(slot.range)}"
+            if len([x for x in sv.class_induced_slots(slot.range) if x.designates_type]) > 0 and len(sv.class_descendants(slot.range)) > 1:
+                return f"Union[" + ",".join([camelcase(c) for c in sv.class_descendants(slot.range)]) + "]"
+            else:
+                return f"{camelcase(slot.range)}"
 
         # For the more difficult cases, set string as the default and attempt to improve it
         range_cls_identifier_slot_range = "str"
diff --git a/linkml/generators/shaclgen.py b/linkml/generators/shaclgen.py
index 06598d8a..76009844 100644
--- a/linkml/generators/shaclgen.py
+++ b/linkml/generators/shaclgen.py
@@ -59,7 +59,10 @@ class ShaclGenerator(Generator):
             class_uri = URIRef(sv.get_uri(c, expand=True))
             shape_pv(RDF.type, SH.NodeShape)
             shape_pv(SH.targetClass, class_uri)  ## TODO
-            shape_pv(SH.closed, Literal(True))
+            if c.mixin or c.abstract:
+                shape_pv(SH.closed, Literal(False))
+            else:
+                shape_pv(SH.closed, Literal(True))
             if c.title is not None:
                 shape_pv(SH.name, Literal(c.title))
             if c.description is not None:
diff --git a/linkml/transformers/relmodel_transformer.py b/linkml/transformers/relmodel_transformer.py
index 377b4e44..d71029ae 100644
--- a/linkml/transformers/relmodel_transformer.py
+++ b/linkml/transformers/relmodel_transformer.py
@@ -445,10 +445,16 @@ class RelationalModelTransformer:
         :param sv:
         :return:
         """
-        pk = SlotDefinition(name="id", identifier=True, range="integer")
+        c = sv.get_class(cn)
+        candidate_names = ["id", "uid", "identifier", "pk"]
+        valid_candidate_names = [n for n in candidate_names if n not in c.attributes]
+        if not valid_candidate_names:
+            raise ValueError(
+                f"Cannot add primary key to class {cn}: no valid candidate names"
+            )
+        pk = SlotDefinition(name=valid_candidate_names[0], identifier=True, range="integer")
         add_annotation(pk, "dcterms:conformsTo", "rr:BlankNode")
         add_annotation(pk, "autoincrement", "true")
-        c = sv.get_class(cn)
         if pk.name in c.attributes:
             raise ValueError(
                 f"Cannot inject primary key {pk.name} as a non-unique attribute with this name already exists in {cn}"
diff --git a/linkml/utils/generator.py b/linkml/utils/generator.py
index a43b2358..c396f9eb 100644
--- a/linkml/utils/generator.py
+++ b/linkml/utils/generator.py
@@ -190,6 +190,7 @@ class Generator(metaclass=abc.ABCMeta):
         else:
             self.schemaview = SchemaView(schema)
             self.schema = self.schemaview.schema
+        self._init_namespaces()
 
     def _initialize_using_schemaloader(self, schema: Union[str, TextIO, SchemaDefinition, "Generator"]):
         # currently generators are very liberal in what they accept, including
@@ -239,10 +240,12 @@ class Generator(metaclass=abc.ABCMeta):
             self.source_file_size = loader.source_file_size
             self.schema_location = loader.schema_location
             self.schema_defaults = loader.schema_defaults
-            if self.namespaces is None:
-                self.namespaces = Namespaces()
-                for prefix in self.schema.prefixes.values():
-                    self.namespaces[prefix.prefix_prefix] = prefix.prefix_reference
+
+    def _init_namespaces(self):
+        if self.namespaces is None:
+            self.namespaces = Namespaces()
+            for prefix in self.schema.prefixes.values():
+                self.namespaces[prefix.prefix_prefix] = prefix.prefix_reference
 
     def serialize(self, **kwargs) -> str:
         """


Test patch:
diff --git a/tests/test_generators/test_sqltransform.py b/tests/test_generators/test_sqltransform.py
index 5db73f6d..89be5c35 100644
--- a/tests/test_generators/test_sqltransform.py
+++ b/tests/test_generators/test_sqltransform.py
@@ -60,11 +60,47 @@ class RelationalModelTransformerTestCase(unittest.TestCase):
         """
         test conflict with injected primary key
         """
-        b = SchemaBuilder()
-        slots = ["id", "description"]
-        b.add_class(DUMMY_CLASS, slots)
-        with self.assertRaises(ValueError):
-            results = self._translate(b)
+        for c_has_pk, d_has_pk in itertools.product((True, False), repeat=2):
+            with self.subTest():
+                name_slot = SlotDefinition("name")
+                b = SchemaBuilder(name=f"test_conflicting_primary_key_name_{c_has_pk}_{d_has_pk}")
+                b.add_class("c",
+                            use_attributes=True,
+                            slots=[
+                                SlotDefinition("id", identifier=c_has_pk),
+                                name_slot,
+                                SlotDefinition("has_ds",
+                                                singular_name="has_d",
+                                                range="d",
+                                                multivalued=True,
+                                                inlined=False)])
+                b = b.add_class("d",
+                                use_attributes=True,
+                                slots=[
+                                    SlotDefinition("id", identifier=d_has_pk),
+                                    name_slot
+                                    ])
+                rel_schema = self._translate(b).schema
+                rsv = SchemaView(rel_schema)
+                c = rsv.get_class("c")
+                d = rsv.get_class("d")
+                c_has_d = rsv.get_class("c_has_d")
+                if c_has_pk:
+                    self.assertCountEqual(["id", "name"], list(c.attributes.keys()))
+                    self.assertCountEqual(["id"], get_primary_key_attributes(c))
+                else:
+                    self.assertCountEqual(["id", "uid", "name"], list(c.attributes.keys()))
+                    self.assertCountEqual(["uid"], get_primary_key_attributes(c))
+                if d_has_pk:
+                    self.assertCountEqual(["id", "name"], list(d.attributes.keys()))
+                    self.assertCountEqual(["id"], get_primary_key_attributes(d))
+                else:
+                    self.assertCountEqual(["id", "uid", "name"], list(d.attributes.keys()))
+                    self.assertCountEqual(["uid"], get_primary_key_attributes(d))
+                c_id_col = "id" if c_has_pk else "uid"
+                d_id_col = "id" if d_has_pk else "uid"
+                self.assertCountEqual([f"c_{c_id_col}", f"has_d_{d_id_col}"], list(c_has_d.attributes.keys()))
+                self.assertCountEqual([f"c_{c_id_col}", f"has_d_{d_id_col}"], get_primary_key_attributes(c_has_d))
 
     def test_no_inject_primary_key(self):
         """
diff --git a/tests/test_issues/test_pydantic_polymorphism.py b/tests/test_issues/test_pydantic_polymorphism.py
new file mode 100644
index 00000000..3ed78092
--- /dev/null
+++ b/tests/test_issues/test_pydantic_polymorphism.py
@@ -0,0 +1,120 @@
+import  unittest
+
+from linkml.generators.pydanticgen import PydanticGenerator
+from tests.test_issues.environment import env
+from tests.utils.test_environment import TestEnvironmentTestCase
+from linkml_runtime.utils.compile_python import compile_python
+
+schema_str = """
+id: http://example.org
+name: inline-dict-test
+imports:
+  - https://w3id.org/linkml/types
+prefixes:
+  x: http://example.org/
+default_prefix: x
+default_range: string
+description: test
+
+classes:
+  NamedThing:
+    slots:
+      - id
+      - full_name
+      - thingtype
+  Person:
+    is_a: NamedThing
+    slots:
+      - height
+  Organisation:
+    is_a: NamedThing
+    slots:
+      - number_of_employees
+  Container:
+    tree_root: true
+    slots:
+      - things
+  ContainerWithOneSibling:
+    slots:
+      - persons
+slots:
+  id:
+    identifier: true
+    range: string
+    required: true
+  thingtype:
+    designates_type: true
+    range: uriorcurie
+  full_name:
+    range: string
+  height:
+    range: integer
+  number_of_employees:
+    range: integer
+  things:
+    range: NamedThing
+    multivalued: true
+    inlined_as_list: true
+  persons:
+    range: Person
+    multivalued: true
+    inlined_as_list: true
+"""
+
+data_str = """
+{
+  "things": [
+    {
+      "id": 1,
+      "thingtype": "x:Person",
+      "full_name": "phoebe",
+      "height": 10
+    },
+    {
+      "id": 2,
+      "thingtype": "x:Organisation",
+      "full_name": "University of Earth",
+      "number_of_employees": 2
+    }
+  ]
+}
+"""
+
+
+class PydanticPolymorphismTestCase(TestEnvironmentTestCase):
+    env = env
+
+    def test_pydantic_obey_range(self):
+        gen = PydanticGenerator(schema_str)
+        output = gen.serialize()
+        
+        self.assertNotRegexpMatches(output, "Union\[[a-zA-Z0-9]*\]", "a python Union should always have more than one option")
+
+        output_subset = [line for line in output.splitlines() if "thingtype" in line]
+        self.assertGreater(len(output_subset), 0)
+        
+        self.assertTrue("const=True" in output_subset[0])
+        self.assertEqual(len([x for x in output_subset if 'x:Person' in x]),1)
+
+        gen = PydanticGenerator(schema_str.replace("uriorcurie","uri"))
+        output = gen.serialize()
+        output_subset = [line for line in output.splitlines() if "thingtype" in line]
+        self.assertGreater(len(output_subset), 0)
+        self.assertTrue("const=True" in output_subset[0])
+        self.assertEqual(len([x for x in output_subset if 'http://example.org/Person' in x]) ,1)
+
+    def test_pydantic_load_poly_data(self):
+        gen = PydanticGenerator(schema_str)
+        output = gen.serialize()
+        mod = compile_python(output, "testschema")
+        data = mod.Container.parse_raw(data_str)
+        
+        self.assertEqual(len([x for x in data.things if isinstance(x,mod.Person)]),1)
+        self.assertEqual(len([x for x in data.things if isinstance(x,mod.Organisation)]),1)
+
+
+
+
+
+if __name__ == "__main__":
+    unittest.main()