Add Flask 2.x compatibility + fix line number detection under python <= 3.8 and python 3.11

[ValentinFrancois]

Issue 1

Cause

When installing Flask 2.0 with python > 3.6, the chosen version of its Jinja2 dependency is >= 3.1.0:

$ docker run -it --rm python:3.7 bash
root@44ff1cecc27d:/# pip3 install Flask==2.0
Collecting Flask==2.0
  Downloading Flask-2.0.0-py3-none-any.whl (93 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 93.2/93.2 KB 2.5 MB/s eta 0:00:00
Collecting Jinja2>=3.0
  Downloading Jinja2-3.1.2-py3-none-any.whl (133 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 133.1/133.1 KB 7.5 MB/s eta 0:00:00

With python 3.6, the installed Jinja2 dependency seems to be < 3.1.0 which should not be problematic as I'll explain next.

$ docker run -it --rm python:3.6 bash
root@7a7dccf967f2:/# pip3 install Flask==2.0
Collecting Flask==2.0
  Downloading Flask-2.0.0-py3-none-any.whl (93 kB)
     |████████████████████████████████| 93 kB 1.6 MB/s 
Collecting Jinja2>=3.0
  Downloading Jinja2-3.0.3-py3-none-any.whl (133 kB)
     |████████████████████████████████| 133 kB 6.4 MB/s 

Consequences

import flask_selfdoc will then raise an ImportError because it tries to import the follow function and class that were deprecated in Jinja2 3.0 and removed in Jinja2 3.1:

• jinja2.evalcontextfilter -> moved to jinja2.pass_eval_context
• jinja2.Markup -> moved to jinja2.utils.markupsafe.Markup

Issue 2

Cause

The way that the line number of a decorator is detected changed across python versions:

• python <= 3.8: stack()[1].lineno points to the line above the decorated function (= to the closest decorator, not necessarily the one that did the call to stack())
• python 3.9 and 3.10: stack()[1].lineno points to the line of the decorated function
• python 3.11: stack()[1].lineno points to the exact line of the decorator that did the call to stack()

Example:

    1   |def call_stack_and_get_lineno():
    2   |
    3   |    def decorator(func):
    4   |        calling_frame = stack()[1]
    5   |        print(calling_frame.lineno)
    6   |        return func
    7   |
    8   |    return decorator
    9   |
    10  |
    11  |@decorator1
    12  |@call_stack_and_get_lineno
    13  |@decorator2
    14  |def func():
    15  |    pass

• python <= 3.8: will print line 13
• python 3.9 and 3.10: will print line 14 (desired behaviour)
• python 3.11: will print line 12

Consequences

• unit tests don't pass for python 3.11
• unit tests pass for python <= 3.8 but only because we adjust them (add a -1 offset to the detected line number)

What this PR does

Issue 1

• Avoid Jinja2-related ImportError by trying the old and the new import for each Jinja function/class that is now deprecated
• Remove the check Flask < 2.0 from the package dependencies
• Adapt the ranges of tested python and Flask versions in the unit tests (WIP)
  • minimal-test.yml: remove python 3.7 (security support ends in 4 months), add python 3.10
  • test.yml: add Flask 2.1 (not compatible with python 3.6), add python 3.10 and 3.11 (not compatible with Flask 1.0)
  • Adapt the import of the Flask app context to version 2.3 where _app_ctx_stack is removed
  • Install target Flask version for tests using pip3 (easier than with poetry) and manually override its dependency versions when they're too recent (itsdangerous, Jinja2 and MarkupSafe for Flask 1.X)
  • List all installed packages before running tests

    Issue 2

• Fix line number detection by reading the source file and finding the next line starting with def

[jwg4]

Thanks for this.

I am very in favour in principle; I want to make sure that we do this correctly and that we don't needlessly limit compatibility with older versions.

[ValentinFrancois]

@jwg4 cool, I clarified the PR description a bit.

I think I spotted an issue in minimal-test.yml and test.yml that was overwriting the target Flask version from the matrix.

I also added python 3.10 to see which is the oldest supported version of Flask (I already excluded 1.0 based on your README.md).

[jwg4]

1. looks good in principle
2. thank you for sorting out the jinja2 mess which I was dreading!
3. removing Python 3.7 from the quick test matrix seems fine.
4. adding flask and python versions to the main test matrix and specifying not to run certain toxic combinations also seems like the right decision.
5. I wonder if we should add 'latest' to the Python version specs, any thoughts?
6. as you can see above, some of the combinations fail - it seems that poetry will not allow you to specify a version which has a more restrictive Python version than what you have set for your own project, even if your Python version matches both specs. This seems right in principle, but I wonder if there's a way of overriding it for these purposes?
7. the fact that 6 has only now come up suggests that you are right about the flask versions not actually being correct before your fix. I think this should be merged as soon as we can find a solution or a workaround for 6.

[jwg4]

thank you so much for this, I have left several comments. let me know if any that don't make sense or you don't think belong to this change.

as you will see, some of them relate to the failing tests which include several causes

[ValentinFrancois]

@jwg4 sorry I didn't see your previous comments while I was still struggling to solve the issues with poetry & dependency handling. I think I've come to a good solution, I left a few comments explaining my recent changes.

The only thing I'm not sure about is why the output of flask_selfdoc gets different line numbers depending on the installed python/Flask versions. For now I added some pre-processing function to remove the line numbers from the compared outputs in test_check_example.py but maybe you'll want to investigate this deeper.

[ValentinFrancois]

I tried adding tests for latest python as you suggested (3.x, currently evaluating to 3.11.2) but under this version, the unit tests for line numbers don't pass:

So I think there's something defninitely weird with inspect and how it calculates line numbers of caller frames. Even with a specific python version, the tests can fail depending on the OS because of different line numbers:

I'm removing the code I had added to ignore these line number differences, because it seems to be more important than I first thought. By the way, I rewrote test.yml to install the target Flask version with pip3, which in the end was easier to use (especially under windows).

[jwg4]

My understanding is that the code which retrieves the line number where a function is defined has changed to take decorators into account in a different way. I believe this is a change in a python built-in which flask uses and does not have control over. I don't know how OS type affects this.

[ValentinFrancois]

Interesting, now python 3.11 seems to have changed again the way line number is calculated.

In this unit test, the frame now points to this line:

I suppose this is related to this: https://docs.python.org/3/whatsnew/3.11.html#whatsnew311-inspect but they didn't detail this behavior.

Anyway I figured out we could actually just read the source file and offset the line number by 1 as long as we don't find a line starting with def. Do you think it's a good idea? I suppose it won't work for cythonized code but it's probably ok since this package is meant to document source files, and not files optimized for production?

[ValentinFrancois]

In the end the PR got bigger than I first planned, but all tests are green now! I updated the PR description to my latest changes

[ValentinFrancois]

just made it work again with pytest, I suppose it didn't like the double inheritance trick

[ValentinFrancois]

Sorry I didn't realize that "${{ github.repositoryUrl }}@${{ github.head_ref }}" would not work on your side, because ${{ github.repositoryUrl }} points to the repository where the action is running, and not the repository where the PR comes from.

I switched to "${{ github.event.pull_request.head.repo.git_url }}@${{ github.event.pull_request.head.ref }}", based on https://stackoverflow.com/a/65027014.

Now the check_pip_install tests should always install from the branch in the source repo (forked or not), which should work on your side too.

[ValentinFrancois]

My bad, I forgot to do the same in the install step for Windows. Hoping it's the last iteration

[ValentinFrancois]

Anything still blocking the merge?

I see one job that seems to be stuck for some reason

[ValentinFrancois]

ping

[jwg4]

hi, sorry for delay, I've been very busy with some other work.

[ValentinFrancois]

No worries, just let me know if there's anything else I can help with. I don't really understand why a single job has stayed stuck this whole time:

[jwg4]

do you think that you could rebase and squash your changes?

either into a single commit, into a small number of commits (if it's easy to separate the different changes like that), or failing that, just to remove the changes which were later rolled back or made obsolete by other changes.

after that's done I will give a final review and merge quickly.

[jwaxy]

is there any progress in merging this?

[ValentinFrancois]

Oh thanks for the reminder! Just rebased into a few commits.

[jwaxy]

And btw is this library compatible with flask-restful?

[ValentinFrancois]

No, the auto documentation logic relies on the Flask.route decorator which is not used in flask-restful.