Add deprecations module and add deprecation warnings for pydantic v1

[sneakers-the-rat]

Fix: #1925

I've actually been meaning to experiment with something like this for awhile, so figured i would take a stab at it. I have seen the decorator a la https://github.com/briancurtin/deprecation a lot before, but i don't necessarily like how the deprecation warnings are tied to the decorator itself, and you have to make sure that the module is imported and the relevant section is run to know they exist.

Usage

This makes a single deprecation module where deprecations are defined, and then you have a simple deprecation_warning function that emits the warning and logs that it was emitted wherever it is relevant. So in this case since we aren't deprecating a function or class per se, but a version of a dependency and a value of a parameter, it is easy enough to drop in with those checks where appropriate. A warning is only emitted once bc it's tracked in the EMITTED set.

Testing

The classes in the deprecation module are tested, but i also test that things that are marked as having been removed should be removed in the version that we say they are! This should help us avoid problems like #1940 and force us to keep up to date. This works by ensuring that test_removed_are_removed is run at the very end of testing, where presumably any point where a deprecation warning would have been emitted would have been hit - if it's not, that's a test coverage problem!

CI

To make this work, I had to make some changes to the CI, sorry if this is a lot

• install the dynamic versioning plugin
• fetch more commits, and the tags from the upstream repo
• install the package instead of using --no-root

Otherwise linkml thinks it is at version 0.0.0 and you can't test whether deprecations post that version have been done

Docs

Made a docs page that looks like this:

Added some dummy/test deprecations just to make sure the plot works, but anyway ya that should make it good and obvious what is getting deprecated when.

I have been wanting to do this both for some of my projects and to demo to my lab, and it turns out that just using jinja is way easier than trying to make a custom sphinx directive. i still have never figured out how that's supposed to work.

Caveats

• I don't necessarily love the use of module variables here, but i figure it's already overkill, and it sets us up with a tool we can use going forward.
• I added sphinx-jinja and matplotlib to the dev dependencies. sry about that. I could also split up the docs dependencies from the other dev dependencies if that would help soothe the additional weight
• the dependency cache on CI is only invalidated with the lockfile changes. since we don't always refresh the lockfile when the version is incremented, we might miss some deprecations until it is. imo we should always refresh the lockfile so that our testing env doesn't get out of sync with what someone would experience when installing from pip, but that's a separate conversation for a different PR!

[codecov]

Codecov Report

Attention: Patch coverage is 97.54098% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 79.89%. Comparing base (4f5e934) to head (faef704).
Report is 1 commits behind head on main.

Files	Patch %	Lines
linkml/generators/pydanticgen/pydanticgen.py	71.42%	1 Missing and 1 partial ⚠️
linkml/utils/deprecation.py	99.11%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1957      +/-   ##
==========================================
+ Coverage   79.72%   79.89%   +0.17%     
==========================================
  Files         107      109       +2     
  Lines       11979    12101     +122     
  Branches     3422     3448      +26     
==========================================
+ Hits         9550     9668     +118     
- Misses       1854     1856       +2     
- Partials      575      577       +2     

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

[sneakers-the-rat]

Somehow 3.8 is failing with the pydantic generator, how did that not happen on the PR? According to the docs for 3.9:

Changed in version 3.9: Class methods can now wrap other descriptors such as property().

which is news to me. i'm going to mark this as ready for review bc aside from that it's fine, and we're in the process of dropping 3.8 anyway in #1704

edit: wait no that was 3.7. OK i'll fix for 3.8 tmrw but still the substance of this PR is ready for review.

[turbomam]

Thanks @sneakers-the-rat . Some of us are causally reviewing this in the LinkML Developers meeting now.

There's a question about whether adding a visualization library like matplotlib is really necessary to communicate the version vs. functionality deprecation timeline.

Can you same some more about the different colors and shapes in your sample deprecation visualization? If color and shape aren't hard requirements for communicating the important points, then maybe it could be communicated with a table or ASCII art?

[sneakers-the-rat]

Haha oh boy I tried to do it with svg for a few hours to avoid adding deps before deciding I should just make a simple version that works and talk about deps later.

The dependency should just be necessary when building docs, and so in general I think of docs dependencies as being much freer than main or even testing deps. Matplotlib is heavy, yes, and a PNG is not ideal format for accessibility either. I didnt want to do too much here but imo we should split up the dev dependencies group into tests and docs at least, so then when running tests/CI no matplotlib or Sphinx etc and vice versa.

I think the visualization would be a really nice touch, and imo worth the docs dependency, but im also super flexible on it. The goal was to have a single page where people could go to see at a glance what to prepare for, with additional information below. This is me "making the deprecation warnings I want to see in the world," so a bit of an experiment (vs. just using the one standard deprecations package)

The rest of the page is effectively table-like, but a sort of collapsed table that can be read on mobile since the deprecation warnings also have potentially paragraph length explanations and recommendations. The plot is useful to visualize relative timing of deps, lengths between deprecations and removals (eg. Pydanticgen support ends sooner than us cranking up the dependency spec), and also shows you that in relation to the current version. I think a table version of that would be comparatively awkward. An ASCII art version would be retro and very cool but potentially a good bit of work to make.

Colors are
green: still good to use
red: stop using - pay attention to this one
gray: already removed.

the lines vs diamonds are just to handle times when we will potentially not set a removal version (eg. To accommodate existing informal deprecations), but maybe that should just be a line that goes arbitrarily to the right.

If theres an easier way to communicate relative position and duration of deps vs. Current version then great! But I think this might also be an opportunity to split up deps so that we dont have to be limited for docs dependencies in the same way we want to be careful for runtime dependencies :)

[amc-corey-cox]

I typed this all out but forgot to post it on Friday:
I took a thorough look at this and unfortunately, I don't think I quite understand the parts of the code it is touching well enough to offer a review. However, I didn't see anything that jump out at me as concerning. Quite a lot of the files/lines touched are just introducing this throughout where it needs to be used. It looks like a good PR to me.

However, I do have a couple of questions about implementation. I'm not entirely sure what the plot adds to the deprecation documentation. It certainly looks nice and maybe it's more useful than I realize but it seems to me we might be able to get the same information across with a simple table. I don't have strong feelings either way on including this I just wanted to mention I'm not sure it is necessary. I also didn't find it obvious what all of the different features mean, i.e. the green diamond and the importance of green/red/gray lines.

[sneakers-the-rat]

How about this. Look at the image above of the page. If that plot adds nothing of value - ie. It doesnt help you know when there are things that will be deprecated, then we can take it out. The section below it is all that information in a tabular form.

In my opinion the plot adds a great deal of "at a glance" clarity - I can tell what version im at now, green is a universal signifier for "ok," red is a universal signifier for "not ok," and then I can see matched headings that give me further information below. The page without the plot would be what it would look like with just "tabular form" information - lots to parse, but possible.

If we dont want it, fine, I dont really care! Main thing i need is someplace to go on this - I want to get it merged in some form so we can actually stay true to the deprecation tl and give advance warning, so just lmk which to do.

[sneakers-the-rat]

The plot also can be changed, annotated, etc. So if thats what is missing that can also be done

[amc-corey-cox]

I don't hate the plot. It does add a certain at-a-glance value. I just thought we should think about it a little before adding more dependencies for a fairly limited application. I'm ambivalent on keeping the plot so I'm not concerned if we do.

[sierra-moxon]

@sneakers-the-rat - we talked in dev about this again - and while we love the idea, we don't love the idea of supporting the utils method added here, just because we're limited in resources. We wonder if you had thought about making this a standalone module? Then we could get feedback from the wider python community, etc? We think it should be some sort of standard.

I'd like to at least turn this into a draft PR, or better into a discussion so we don't lose your work, but we aren't ready to bring it in yet.

[sneakers-the-rat]

I don't hate the plot. It does add a certain at-a-glance value. I just thought we should think about it a little before adding more dependencies for a fairly limited application.

again, not adding a runtime dependency, this would be a dependency that is only necessary when building the docs, i suggested above that we split up dev-dependencies into docs and tests so one wouldn't even need to install it during development. pretty much the only time matplotlib would need to be installed are the two CI actions that build the docs.

edit: also if we really hate it, i could render the plot to SVG and then write the modification code separately and use a jinja template to render it. it would be harder to change in the future but wouldn't require matplotlib

---

we don't love the idea of supporting the utils method added here

you mean the whole deprecation.py module?

We wonder if you had thought about making this a standalone module? Then we could get feedback from the wider python community, etc? We think it should be some sort of standard.

sure, could definitely do that, but then it would just be a dependency which seems like more of a maintenance burden IMO?

I don't really care about the form of this PR - we can totally ditch the plot, we can make it a separate package, all fine, but I guess stepping back i'd be curious what y'all do want as far as deprecation warnings go? there is a bunch of refactoring work to be done, and i'd like to help with that, and to do that in a responsible way we should be leaving deprecation warnings as we go and doing gradual phase-outs.

Elsewhere I've noticed a bunch of places where stuff is soft marked as deprecated but ends up hanging around for awhile (eg. #1940 and the validators module

linkml/linkml/validator/__init__.py

Line 3 in 6d739f7

than the ``linkml.validators`` package. While that package still exists, it may become deprecated

), so it seems like we are holding on to old code because we don't have a good deprecation system. So the purpose of this PR was to make a central place to keep track of and enforce deprecation so that we can iteratively streamline the package to decrease, rather than increase maintenance burden.

So eg. here: #1967 there's some consolidation that should be done that probably includes deprecating importing something from the current location either in linkml or linkml-runtime (see linkml/linkml-runtime#305 (comment) ). with this PR, we would be able to mark that here:

linkml/linkml/validator/loaders.py

Lines 52 to 58 in f5010ba

	if name in deprecation_maps.keys():
	warnings.warn(
	(
	f"Importing from {name} is deprecated, use {deprecation_maps[name]} instead."
	"Loaders have been moved to linkml_runtime.loaders"
	)
	)

like:

DEPRECATIONS = (
  # other ones...
  Deprecation(
    name="runtime-loaders",
    deprecated_in=SemVer.from_str("1.7.5"),
    removed_in=SemVer.from_str("1.8.0"),
    message="message explaining the change"
    # ...
  )
)

and then replace that warning with

deprecation_warning('runtime-loaders')

and then that deprecation becomes visible across the project. Currently it's sort of challenging to keep track of what and where things are being changed aside from keeping on top of all the PRs that are happening, but this PR lets me easily add a clear, global signal about an incoming change. Otherwise you would only see that deprecation warning in tests, which not everybody runs, and so someone else working on a different part of the package could be surprised when it's gone.

---

So again i'm down to do whatever, but I'm not really sure what the hesitance is with this module in particular - it's fully tested, relatively straightforward, isolated, and has no downstream utility (eg. we won't have people raising issues because they are trying to use it in their project). LMK what would be good to do bc linkml needs some deprecation system and i thought this was a relatively tidy way to do that.

[sneakers-the-rat]

ping on this - list of list array PR is almost done, and i imagine that might be a minor version bump. we had set a tentative 1.8.0 deprecation date for pydantic generator but we haven't even deployed a warning yet, so that'll probably have to get pushed back, but ya ideally not much further bc i want to be able to simplify that generator at some point. still unsure what the hesitation is!

[sneakers-the-rat]

pinging again on this - seems like we are moving closer and closer to pydantic v2 being default, and it would be really nice to give a long deprecation timeline to ppl who are still using pydantic 1 models. if not this PR, then lmk what we would want as far as deprecation warnings - this PR is basically a set of nice things that make it easier for us to manage the many deprecations that currently are leaving zombie code in the library or we might be hesitant to do because we don't have a good system for deprecations! but if we don't like this, then we can do something else, i'm just not really sure what the objections are and so i don't know what else to propose!

[sierra-moxon]

from dev meeting:

• @sneakers-the-rat evaluated deprecation annotations/decoration (in use in linkml-runtime already) - https://pypi.org/project/deprecation/
  • the documentation generator needs to know about those annotations vs. just encountering this at runtime.
• instead thinking about a Deprecation object, with methods that call it/display its output
• this would be helpful for non-method-level deprecation.
• we'd like python to give us a standard here, instead of rolling this ourselves, e.g. same as java.
• might be something in typing that allows marking as deprecated.
• scipy has one (in repo - not genericizable)
• can this live as its own thing and generate a community around it? (then we can import it as a dependency).

decision:

• move forward with this
• @sneakers-the-rat will write some docs, and fix merge conflicts and will review.

[sneakers-the-rat]

OK Docs pushed! here's a rendered copy for ur perusal

deprecation.html.zip

[sneakers-the-rat]

It looks like using the setup-python action with the poetry cache can be used without doing --no-root (which previously could cause an old version of the package to be installed): https://github.com/actions/setup-python/blob/main/docs/advanced-usage.md#caching-packages

and we need the version info for linkml to be able to test that things marked as removed are actually removed, so i changed to install whole package.

edit: and also edited the pydantic 1 tests to match

[sneakers-the-rat]

wat in tarnation did I break. Need to do other stuff for now so marking as draft until I get back to it

[sneakers-the-rat]

ok that was weird but were good now. redy for review

[sierra-moxon]

Thank you for the documentation @sneakers-the-rat - I appreciate it very much.