ENH: drop python 3.8, 3.9 and support 3.13

[jacopoabramo]

Description

Closes #367 and #354

Motivation and Context

Python 3.9 reaches EOL end of this month; this is an attempt to mirror bluesky repo.

Currently blocked by #1959

How Has This Been Tested?

Tested locally but some changes due to autogenerating typed dictionary models are fishy.

[jacopoabramo]

@evvaaaa any suggestions? Apparently some fields have been changed from NotRequired to Literal during the generation process (after setting the minimum python version to 3.10).

[evvaaaa]

This would be nice, however, bluesky supports 3.9 still, and event-model is a required dependency. I wouldn't be happy updating this without also dropping 3.9 from bluesky.

[jacopoabramo]

This would be nice, however, bluesky supports 3.9 still

There's a PR open for that. Once that's merged, this can be merged as well safely I believe

[evvaaaa]

To be merged alongside the bluesky update.

[jacopoabramo]

Happy new year @evvaaaa ; just a friendly ping that #1959 has been merged.

[evvaaaa]

Happy new year @evvaaaa ; just a friendly ping that bluesky/bluesky#1959 has been merged.

Happy new year to you too 🥳

Cool, happy for this to be merged. Can you rebase and squash it?

[jacopoabramo]

@evvaaaa it seems the latest release of datamodel-code-generator is switching entirely to ruff for formatting; is it ok for me to adapt that change to this PR?

EDIT: according to the error message the warning can be suppressed by explicitly setting the formatters in the generate function. Since event-model is already using ruff, I can simply add it in the API call

[jacopoabramo]

Ah, nevermind, the test actually fails because PydanticDeprecatedSince20 apparently is not raised... but we could it two birds with one stone

[evvaaaa]

Some of our changes to datamodel-code-generator have also been merged. I can make an issue, will let us cut down on custom jinja.

For now we just fix it by doing what they suggest.

[jacopoabramo]

@evvaaaa I applied their suggestion locally; trouble is that it produces a series of exceptions. Copy-pasting the local output of my tox -e tests command:

  + Exception Group Traceback (most recent call last):
  |   File "C:\git\event-model\.venv\lib\site-packages\_pytest\runner.py", line 353, in from_call
  |     result: TResult | None = func()
  |   File "C:\git\event-model\.venv\lib\site-packages\_pytest\runner.py", line 245, in <lambda>
  |     lambda: runtest_hook(item=item, **kwds),
  |   File "C:\git\event-model\.venv\lib\site-packages\pluggy\_hooks.py", line 512, in __call__
  |     return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  |   File "C:\git\event-model\.venv\lib\site-packages\pluggy\_manager.py", line 120, in _hookexec
  |     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  |   File "C:\git\event-model\.venv\lib\site-packages\pluggy\_callers.py", line 167, in _multicall
  |     raise exception
  |   File "C:\git\event-model\.venv\lib\site-packages\pluggy\_callers.py", line 139, in _multicall
  |     teardown.throw(exception)
  |   File "C:\git\event-model\.venv\lib\site-packages\_pytest\logging.py", line 858, in pytest_runtest_teardown
  |     yield
  |   File "C:\git\event-model\.venv\lib\site-packages\pluggy\_callers.py", line 139, in _multicall
  |     teardown.throw(exception)
  |   File "C:\git\event-model\.venv\lib\site-packages\_pytest\capture.py", line 905, in pytest_runtest_teardown
  |     return (yield)
  |   File "C:\git\event-model\.venv\lib\site-packages\pluggy\_callers.py", line 121, in _multicall
  |     res = hook_impl.function(*args)
  |   File "C:\git\event-model\.venv\lib\site-packages\_pytest\unraisableexception.py", line 163, in pytest_runtest_teardown
  |     collect_unraisable(item.config)
  |   File "C:\git\event-model\.venv\lib\site-packages\_pytest\unraisableexception.py", line 81, in collect_unraisable
  |     raise ExceptionGroup("multiple unraisable exception warnings", errors)
  | exceptiongroup.ExceptionGroup: multiple unraisable exception warnings (10 sub-exceptions)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "C:\git\event-model\.venv\lib\site-packages\datamodel_code_generator\format.py", line 347, in format_code
    |     code = self.apply_ruff_check_and_format(code)
    | ResourceWarning: unclosed file <_io.BufferedReader name=17>
    |
    | The above exception was the direct cause of the following exception:
    |
    | Traceback (most recent call last):
    |   File "C:\git\event-model\.venv\lib\site-packages\_pytest\unraisableexception.py", line 67, in collect_unraisable
    |     warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))
    | pytest.PytestUnraisableExceptionWarning: Exception ignored in: <_io.FileIO name=17 mode='rb' closefd=True>
    | Enable tracemalloc to get traceback where the object was allocated.
    | See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

The specific exception of the exception group is raised 10 times in total (a.k.a. for each JSON schema document currently generated, except for the bulk_ documents which are deprecated anyway). From the looks of it, it tries to run ruff check/format as a subprocess on an currently opened file and it can't apply modifications. I'm not sure if this is a bug on datamodel or in the our generation script; to keep things simple I would suggest to filter out the FutureWarning produced and notify this behavior. Since it's not inherent to the generation process and we run ruff regardless of the outcome it should not cause issues.

[jacopoabramo]

I suppressed the warning on the generate function, so that it does not propagate when running python -m event_model.generate; I understand it's not the optimal choice but I would rather get this PR out and try to fix the warning later on.

[jacopoabramo]

The failing type checking test is caused by the fact that the latest version of datamodel-code-generator is issuing the closed parameter to initialize the typed dictionary subclasses to enforce the rule that documents cannot be extended with additional keys (PEP 728). Apparently mypy still doesn't support this feature.

What's your reccomendation? Can this be added as an ignore for mypy?

Although the more I progress with this PR the more troublesome it becomes...

EDIT: maybe it could be solved via editing the ninja template

[jacopoabramo]

I opened #2951 to see how to suppress this. Most probably it will involve adapting the template.

[jacopoabramo]

@evvaaaa I can vendor the original TypedDict jinja template from datamodel. I would still prefer the CLI option nevertheless; the only concern I have is that the template has to pin-point typed dictionaries that have no inheritance, since this closed parameter is not generated for typed dictionaries that may be used as a baseline to extend a document.

I get the impression that closed=True might be caused by the presence of "additionalProperties": false in the specific schemas (i.e. the conversion from run_start.json produces a closed typed dictionary, while event.json does not and at first glance it's their main difference in terms of structure).

[jacopoabramo]

@evvaaaa #2951 has been resolved with datamodel-code-generator>=0.53.0; I removed the template directory and pinned the version of the code generator to stay on the safe side. Feel free to review. The only nitpick is the annoying ignore filter in the generate call but other than that everything checks out.

[evvaaaa]

Thanks for this, turned into a much larger PR than anticipated :)

[evvaaaa]

Maybe do the rebase an push here before merging, I'm not sure what is out of sync 🤔

[jacopoabramo]

Done; last update was 5 months ago anyway... :U

[evvaaaa]

Will do a minor release as soon as bluesky does.