linkml-runtime icon indicating copy to clipboard operation
linkml-runtime copied to clipboard

feat: jsonasobj2 vendoring

Open ticapix opened this issue 10 months ago • 10 comments

As proposed here https://github.com/linkml/linkml/issues/1966

ticapix avatar Jan 13 '25 15:01 ticapix

Any idea how to run the CI locally ?

ticapix avatar Jan 13 '25 18:01 ticapix

Tox is great for testing different Python versions locally. Setting up a local CI for full cross-platform tests is probably not worth the effort.

I updated the outdated tox.ini in #357 to work with current tox (4.23.2), see this commit: https://github.com/linkml/linkml-runtime/pull/357/commits/822eb5ed28b158531bc11ae2ca7b08bb9aa10818

dalito avatar Jan 13 '25 18:01 dalito

Can we add deprecation warnings to all the jsonasobj2 behavior while we do this? We want to say "hey if your code relies on using the special jsonasobj stuff, these will soon become just regular dataclasses, see xyz page in the docs for an upgrade guide" with plenty of lead time. That should be visible even if people don't regenerate models during the deprecation window if they don't have an upper bound on linkml-runtime in CI.

I think we would want that on the dunder methods in the jsonobj class, as well as in every function like as_dict etc. We dont have the nice tidy deprecation warning system from main linkml package here so we will be emitting redundant warnings and will have to just deal with the noise until we finish removing jsonasobj, but imo it's better for us to deal with some noise than downstream ppl being caught by surprise

sneakers-the-rat avatar Jan 13 '25 20:01 sneakers-the-rat

Tox is great for testing different Python versions locally. Setting up a local CI for full cross-platform tests is probably not worth the effort.

I updated the outdated tox.ini in #357 to work with current tox (4.23.2), see this commit: 822eb5e

image

ticapix avatar Jan 13 '25 20:01 ticapix

@sneakers-the-rat like that 185e6ed ?

I used typing_extensions.deprecated - which is already a dependency - on all methods not starting with an _, and yes ... running the tests, it's bit verbose :)

============================================= 384 passed, 278 skipped, 1 xfailed, 301698 warnings in 52.26s ==============================================

ticapix avatar Jan 13 '25 21:01 ticapix

Haha oh boy, ya that might be too verbose. maybe we emit one deprecation warning when importing that module?

sneakers-the-rat avatar Jan 14 '25 00:01 sneakers-the-rat

@sneakers-the-rat

(few hours of sleep later) fixed in 73effc9 by using warning.warn at the top of the module and switch to PendingDeprecationWarning

====== 384 passed, 278 skipped, 1 xfailed, 46 warnings in 52.32s ===========

ticapix avatar Jan 14 '25 09:01 ticapix

Drive by peeking at this PR as I'm working on another linkml-runtime release. Since we've picked up a fair number of conflicts here as other stuff merges in, can we:

  • [ ] turn @ticapix's work here into a branch we can store in our repo so we can pick up on @sneakers-the-rat's comments and keep going here.
  • [ ] close this PR once the branch is made (pull in @ticapix's work directly to maintain attribution above)
  • [ ] make sure the new branch references this issue: https://github.com/linkml/linkml/issues/1966 , e.g. issue-1966 so we keep it around as a branch until we're ready to finish the work.

@amc-corey-cox

sierra-moxon avatar May 02 '25 16:05 sierra-moxon

I started this PR to address bugs in the RDF serialisation and python module generation. In the end, I wrote my own pydantic generator from a linkml SchemaView object, with builtin support for RDF serialisation/deserialisation to an rdflib.Graph() object.

Unfortunately, I don't plan to work on this PR anymore and it can be closed.

ticapix avatar May 02 '25 20:05 ticapix

Can you link that @ticapix ? Im curious and want to see it :)

aside on needing to duplicate forked branches to work on them

I have said this a few times, so sorry if this is getting lost in the mail, but in general we shouldn't need to duplicate PRs and branches if they have allow edits from maintainers enabled. Since the author in this case is bowing out (totally fine), that would make sense to let someone else take over. It's almost equivalent to making a new branch just preserves history and reduced clutter. Not a strong preference, just mentioning it because this seems to come up a lot

git remote add ticapix https://github.com/ticapix/linkml-runtime
git fetch ticapix
git checkout -b feat/jsonasobj2_vendoring ticapix/feat/jsonasobj2_vendoring

sneakers-the-rat avatar May 03 '25 01:05 sneakers-the-rat