maint(test): add optional dependency tests to actions

[oscarbenjamin]

References to other Issues or PRs

See #20374

Brief description of what is fixed or changed

Adds optional dependency tests to github actions.

Other comments

Release Notes

NO ENTRY

[sympy-bot]

✅

Hi, I am the SymPy bot (v161). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

• No release notes entry will be added for this pull request.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->

See #20374

#### Brief description of what is fixed or changed

Adds optional dependency tests to github actions.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
NO ENTRY
<!-- END RELEASE NOTES -->

[asmeurer]

FWIW I think even with GHA it would be simpler to keep the bulk of the CI logic in a separate script like we have with test_travis.sh. It's much easier to deal with bash logic in a proper bash script than in a YAML file.

[asmeurer]

I guess doing it this way causes things to split up in the logs. I wonder if there's a way to get the best of both worlds with this.

[asmeurer]

I guess for stuff like this that's "oneliners" we can put it in the YAML file, but for longer stuff, like most of what is still in test_travis.sh, we should keep a bash script.

[asmeurer]

Also worth noting (from https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#jobsjob_idsteps):

Each step runs in its own process in the runner environment and has access to the workspace and filesystem. Because steps run in their own process, changes to environment variables are not preserved between steps. GitHub provides built-in steps to set up and complete a job.

So things should only be split out like this if they really are separate.

[oscarbenjamin]

FWIW I think even with GHA it would be simpler to keep the bulk of the CI logic in a separate script like we have with test_travis.sh. It's much easier to deal with bash logic in a proper bash script than in a YAML file.

I could make a bin/test_optional_dependencies.py. Personally I think that the travis.sh/.travis.yml set up is a mess that has only built up that way because Travis uses environment variables to identify different jobs. The run: key in actions makes it much clearer what commands are being run in each job.

[asmeurer]

If .py seems better than .sh that's fine. The main thing I take issue with is having complex code in a YAML file. Actually test_travis already contains a lot of Python in strings, which has the same problems. So moving it to a Python script is probably better anyway.

[codecov]

Codecov Report

Merging #20532 (1fa5396) into master (723630c) will increase coverage by 0.020%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##            master    #20532       +/-   ##
=============================================
+ Coverage   75.741%   75.762%   +0.020%     
=============================================
  Files          673       673               
  Lines       174437    174499       +62     
  Branches     41200     41205        +5     
=============================================
+ Hits        132122    132205       +83     
+ Misses       36597     36575       -22     
- Partials      5718      5719        +1     

[sympy-bot]

🟠

Hi, I am the SymPy bot (v161). I've noticed that some of your commits add or delete files. Since this is sometimes done unintentionally, I wanted to alert you about it.

This is an experimental feature of SymPy Bot. If you have any feedback on it, please comment at sympy/sympy-bot#75.

The following commits add new files:

• 5451a07:
  • bin/test_optional_dependencies.py
• 1fa5396:
  • bin/test_sphinx.sh

If these files were added/deleted on purpose, you can ignore this message.

[oscarbenjamin]

@asmeurer does this look okay to you?

There is clearly room for improvement in this but just shifting everything over to Actions is top priority right now and this is only one part of that.

[asmeurer]

It looks good. I think Travis still has the dev docs deployment, but is there anything else left? For the time being, I would just disable every build on Travis except for the dev docs deployment build (and make it only run on master and not be required to merge).

[oscarbenjamin]

I think Travis still has the dev docs deployment, but is there anything else left?

I haven't included the docs deployment yet.

The other things not included are:

1. Test on all supported Python versions.
2. Coverage measurement
3. Test Py2 import
4. Benchmarks
5. Tensorflow 1.x
6. Test with pypy

I wonder how many of these we need now. We should certainly test at least the minimum and maximum supported Python versions but we don't necessarily need to test all versions in between.

I think that coverage measurement is useful but also not critical if we're in a rush. I'm not that impressed with codecov so I wonder if there's a better way. We can have artifacts in actions so maybe we should just use those to show diff coverage somehow...

I'm not sure how long we need to keep the Py2 test.

The benchmarks are useful sometimes. I'd like to find a way of making the output from the benchmarks more noticeable. Currently on Travis the benchmarks job always passes. I would like to get to a point where CI picks up on slowdowns but we don't have that with Travis so it can't be critical in the transition to actions.

I think we can just ditch the Tensorflow 1.x tests.

I can't remember the last time I saw anything meaningful from the pypy tests. Maybe they were more useful before but they don't seem that worthwhile to me.

[asmeurer]

PyPy used to fail a lot. I guess that doesn't happen any more?

I agree I personally don't find codecov useful (partly because of #16196), but perhaps others do.

Testing the benchmarks in a way that actually gives useful benchmarks is a harder problem. It might be easier with actions since we can use self hosted runners. I would worry about this later, though. I thought the point of the benchmarks run was just to make sure the benchmarks don't break.

I have no idea what the situation is with tensorflow.

The py2 test is very simple, and I think it's worth keeping for the time being.

I wonder how many of these we need now. We should certainly test at least the minimum and maximum supported Python versions but we don't necessarily need to test all versions in between.

I'm not sure. But we should also consider changing our Python version policy to match something like NEP 19, where we drop Python versions earlier. The issue is that Python has increased the cadence of releases, so our current policy of supporting Python versions that are still supported by Python itself means we now support 5 Python versions at once (see https://www.python.org/dev/peps/pep-0602/).

Regarding docs deployment, I'm working on drdoctr/doctr#372.

[oscarbenjamin]

So the main things needed are:

1. Test all Python versions (and perhaps pypy).
2. Test the Python 2 import.

That should be enough to switch off Travis.

I agree we should follow NEP 19.

Deploying the docs might be easier with actions.