Closed sneakers-the-rat closed 1 month ago
Attention: Patch coverage is 98.33333%
with 1 lines
in your changes are missing coverage. Please review.
Project coverage is 62.80%. Comparing base (
ed36311
) to head (ef2e4d0
). Report is 1 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
linkml_runtime/utils/schemaview.py | 0.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
some kind of weird windows vs ubuntu nondeterminism here...
I like this! As you might expect I'd like to be careful in rolling this out.
As you might expect I'd like to be careful in rolling this out.
always <3
some kind of weird windows vs ubuntu nondeterminism here...
I think windows just always passes (it's doing the same thing over here https://github.com/linkml/linkml-runtime/pull/312#issuecomment-2011143009 )
I probably need to update some string matching tests, will do that
OK so i went to go fix the failing tests, and once i realized what they were doing, rather than updating the string representation i just swapped those out for instance equality checks rather than string equality tests which are a more direct and separate out the string representation from the content of the test.
we have a problem tho, because this doesn't quite fix the problem, which is that whenever there is an error or python shows you the object for some reason, you get a huge blob of empty params and you can't see the salient things for that model. the __str__
override is nice for when you're explicitly print()
ing or str()
ing an object, but the rest of the time python uses __repr__
. unfortunately dataclasses define their own __repr__
method by default, so the YAMLRoot
override doesn't propagate to the children.
SO. this PR is still imo a step in the right direction, and once/if this is merged then I can go and change the pythongen
to add an option for whether or not to set @dataclass(repr=False)
. This is really mostly an issue for the metamodel classes which have a ton of common attributes, so i'll set that default True
(exclude repr) for when generating metamodel classes and False
otherwise.
sound good???
I also updated the pretty print methods to preserve the name of the class, rather than just printing as a dict, so now we have
ClassDefinition({ 'class_uri': 'schema:Person',
'description': 'A person (alive, dead, undead, or fictional).',
'from_schema': 'https://w3id.org/linkml/examples/personinfo',
'is_a': 'NamedThing',
'mixins': ['HasAliases'],
'name': 'Person',
'slot_usage': { 'primary_email': { 'name': 'primary_email',
'pattern': '^\\S+@[\\S+\\.]+\\S+'}},
'slots': [ 'primary_email', 'birth_date', 'age_in_years', 'gender',
'current_address', 'has_employment_history',
'has_familial_relationships', 'has_interpersonal_relationships',
'has_medical_history']})
and once we implement that change to the metamodel generator it should work recursively.
OK this is ready to review, opened a pull to main repo that matches.
the deal witht he complicated print method that isn't just a call to pformat
is that especially with longer schema it's actually extremely difficult to read - pformat hangs the indent of a nested object against the indentation level of its parent, rather than the global depth of nesting. for example this is what a schema looks like with vanilla pformatting
and this is what this PR does:
i actually can't emphasize enough how much of a quality of life improvement this is - it might seem like a trivial thing, but it is one of the very first things i noticed when i was using linkml - never print the models, ever, because they will totally wipe out your shell's traceback buffer and offer nothing of value. when working with small schemas with one or two classes this is totally infuriating because you've only set like 5 properties and somehow are getting like 1000 lines of text back when you're trying to see what the hell anything is.
this pr makes it actually pretty manageable to work with linkml interactively, i was being delightfully surprised while debugging it. i think it's worth the complexity of the manual string formatting i do here
i have been using this locally for working with linkml today and i think y'all are gonna love it. really changes how possible it is to read and debug problems in the code :)
This is really great!
Just one Q - the syntax is a bit of a hybrid between python and a kind of OO-json. I agree a plain json serialization is less desirable as the typing becomes implicit. Any thoughts as to making the output pure python that evaluates to the objects that are printed. This could be done with **s
but that is a bit ugly when just scanning. Alternatively just make this normal instantiation syntax?
Id be fine with either! The instantiation syntax would be tricky bc i am using pformat to handle all the other python objects, so it would be a different kind of hybrid syntax, but adding a **
would be np :)
looks like the upstream tests are confused by the autoversioning...
Yes yes thats this: https://github.com/linkml/linkml-runtime/pull/319
any chance we can merge this pretty plz, i am working on array stuff again and cursing the long print statements while debugging :''(
Here's a drop-in monkeypatch for anyone else affected by this making it super hard to work with linkml models. just call this before you instantiate the models and you shoudl be good
def patch_pretty_print() -> None:
"""
Fix the godforsaken linkml dataclass reprs
See: https://github.com/linkml/linkml-runtime/pull/314
"""
import re
from pprint import pformat
from typing import Any
import textwrap
from dataclasses import is_dataclass, make_dataclass, field
from linkml_runtime.linkml_model import meta
from linkml_runtime.utils.formatutils import items
def _pformat(fields: dict, cls_name: str, indent: str = ' ') -> str:
"""
pretty format the fields of the items of a ``YAMLRoot`` object without the wonky indentation of pformat.
see ``YAMLRoot.__repr__``.
formatting is similar to black - items at similar levels of nesting have similar levels of indentation,
rather than getting placed at essentially random levels of indentation depending on what came before them.
"""
res = []
total_len = 0
for key, val in fields:
if val == [] or val == {} or val is None:
continue
# pformat handles everything else that isn't a YAMLRoot object, but it sure does look ugly
# use it to split lines and as the thing of last resort, but otherwise indent = 0, we'll do that
val_str = pformat(val, indent=0, compact=True, sort_dicts=False)
# now we indent everything except the first line by indenting and then using regex to remove just the first indent
val_str = re.sub(rf'\A{re.escape(indent)}', '', textwrap.indent(val_str, indent))
# now recombine with the key in a format that can be re-eval'd into an object if indent is just whitespace
val_str = f"'{key}': " + val_str
# count the total length of this string so we know if we need to linebreak or not later
total_len += len(val_str)
res.append(val_str)
if total_len > 80:
inside = ',\n'.join(res)
# we indent twice - once for the inner contents of every inner object, and one to
# offset from the root element. that keeps us from needing to be recursive except for the
# single pformat call
inside = textwrap.indent(inside, indent)
return cls_name + '({\n' + inside + '\n})'
else:
return cls_name + '({' + ', '.join(res) + '})'
def __repr__(self):
return _pformat(items(self), self.__class__.__name__)
for cls_name in dir(meta):
cls = getattr(meta, cls_name)
if is_dataclass(cls):
new_dataclass = make_dataclass(cls.__name__,fields=[('__dummy__', Any, field(default=None))], bases=(cls,), repr=False)
new_dataclass.__repr__ = __repr__
new_dataclass.__str__ = __repr__
setattr(meta, cls.__name__, new_dataclass)
same thing here, would like to remove a monkeypatch :)
just rebased this and the main repo PR https://github.com/linkml/linkml/pull/2032 same story as before, main repo tests will fail, manually run the upstream tests against this PR to confirm this won't break stuff!
omg YES!
hi i'm โ๐ธโโ๐นโโ๐ทโโ๐ฎโโ๐ณโโ๐ฌโ โ๐ทโโ๐ชโโ๐ตโโ๐ทโโ๐ชโโ๐ธโโ๐ชโโ๐ณโโ๐นโโ๐ฆโโ๐นโโ๐ฎโโ๐ดโโ๐ณโ โ๐ดโโ๐ซโ โ๐ฒโโ๐ดโโ๐ฉโโ๐ชโโ๐ฑโ โ๐ฉโโ๐ชโโ๐ซโโ๐ฎโโ๐ณโโ๐ฎโโ๐นโโ๐ฎโโ๐ดโโ๐ณโ
you might remember me from other print statements such as
well today i'm here to talk to you about
edited, updated format: