Test older versions of pandas, torch

[isabelizimm]

Intent

Describe briefly what problem this pull request resolves, or what new feature it introduces. Include screenshots of any new or altered UI. Link to any GitHub issues that are related.

Instead of time travel, which might be erratic, specifically choose older major versions of packages to run against.

Versions of packages for which we have specific inspectors:

pandas: has 1.X, 2.X
torch: has 1.X, 2.X
polars: still on 0.X, just test latest
numpy: has been 1.X since 2006, soon to release 2.X

related to #2302 #2597

Approach

Describe the approach taken and the tradeoffs involved if non-obvious; add an overview of the solution if it's complicated.

I've changed the name of the requirements file and specifically pinned older versions of packages. These are both the last release of 1.X versions.

QA Notes

Add additional information for QA on how to validate the change, paying special attention to the level of risk, adjacent areas that could be affected by the change, and any important contextual information not present in the linked issues.

You should be able to test this locally with yarn testMinimumPythonReqs. Note that you must use Python 3.9 or lower, since torch 1.X cannot be installed on later versions.

[isabelizimm]

this is currently testing both of these package versions at the same time, since I don't believe they should interfere with each other. we can split them out if we want, though.

[wesm]

One alternative approach would be to put the pandas / torch / other library versions here instead of in a requirements file. That would make it a bit easier to write down an explicit matrix of library combinations. You would have to have an override step for each supported library (like "Install older pandas version", "Install older torch version", etc.). It wouldn't work for the gulpfile, though

[seeM]

@petetronic had previously mentioned that we may want to ensure that our CI tasks are easily reproducible locally, and I agree that it would be helpful to debug these when they fail on CI.

Since the current matrix is simple, I think this is fine, but if we need more flexibility we could consider using something like nox.

It lets you parametrize sessions, and creates a separate virtual environment for each set of parameters (you can opt out of using a venv with @nox.session(python=False). Very roughly, our noxfile would look like:

@nox.session
@nox.parametrize('pandas', ['1.5.3', 'latest'])
@nox.parametrize('torch', ['1'.12.1, 'latest'])
def tests(session, pandas, torch):
    # TODO: install positron_ipykernel and its dependencies
    # TODO: install base test requirements
    if pandas != 'latest':
        session.install(f'pandas=={pandas}')
    if torch != 'latest':
        session.install(f'torch=={torch}')

Then you could run nox --session tests to run the full matrix, or nox --session "tests(pandas='1.5.3', torch='1.12.1')" for just that case, and so on. The GitHub action could also use a matrix that passes its arguments down to a nox --session call.

There is a noxfile from upstream too, but I'd suggest creating our own specifically for the positron_ipykernel package.

[isabelizimm]

Actually, nox seems like it might be the best of both worlds. The matrix is much easier to parse, and it would easily be put in a gulpfile. Let me see if I can get that set up, since I think it would set us up better long-term.

[seeM]

Does --no-deps mean that we could end up with a pandas version as well as incompatible dependencies of pandas?

[isabelizimm]

Ah, I was under the impression that it just did not give an error if a dependency couldn't be resolved, but it looks like it would not downgrade dependencies as we need it to. I'll use --force-reinstall instead!

[seeM]

My preference would be to repurpose this task to only install the requirements and rename it to installMinSupportedPythonRequirements, then change the package.json script to: gulp installMinSupportedPythonRequirements && pytest pythonFiles/positron. Since IIUC gulp swallows any extra args but yarn passes them through, so I could e.g. do yarn testMinimumPythonReqs --pdb to enable the pytest debugger.

[isabelizimm]

oh cool! I didn't know that yarn passed through args. That is super useful for testing

[seeM]

This is a good step! I'm unsure about the --no-deps arg and left a few other nits/thoughts.

[isabelizimm]

I sort of prefer to have the desired matrix here, and then always run in CI:

yarn positron:testMinimumPythonReqs

but we can also list latest as a version and use

yarn positron:testMinimumPythonReqs --session "tests(torch='1.12.1', pandas='1.5.3')"

[seeM]

LGTM, neat to be trying out nox too! A few more nits - will let you decide whether to implement them