sympy icon indicating copy to clipboard operation
sympy copied to clipboard

[WIP] Make SymPy depend on 3rd party multipledispatch module

Open oscarbenjamin opened this issue 1 year ago • 6 comments

References to other Issues or PRs

An alternative to #23621. This does not yet fix the import time problem though.

Brief description of what is fixed or changed

Change all usage of sympy.multipledispatch to import from and use multipledispatch. Add a dependency on the 3rd party Python multipledispatch module.

There are some incompatibilities between SymPy's multipledispatch and the PyPI package that need to be resolved before fully deleting the module:

  • [X] Change all imports
  • [X] Add dependency in setup.py
  • [X] Install in CI
  • [ ] Make sure all tests pass
  • [ ] Resolve any incompatibilities
  • [ ] Move any code in the sympy.multipledispatch module somewhere else (maybe there could be sympy/external/multipledispatch).
  • [ ] Delete the sympy.multipledispatch module and associated tests
  • [ ] Remove all references e.g. in setup.py, LICENSE, etc
  • [ ] Improve import time
  • [ ] Update docs

Other comments

The 3rd party multipledispatch module does not seem to be actively maintained e.g. it hasn't had a release since 4 years ago in 2018. It's not clear that it really needs much in terms of fixes though because it is relatively simple.

We might not want to depend on it as a 3rd party package but if not then at least we should update the vendored version to match the latest PyPI release because it has a number of improvements. At least there has been one fix in SymPy (#22029) that has not been applied to the 3rd party package https://github.com/mrocklin/multipledispatch/issues/118.

Alternatively if it does need more maintenance maybe it would be possible to take over maintenance.

Release Notes

  • other
    • SymPy now depends on the third party multipledispatch module (as installed by pip install multipledispatch) rather the vendoring its own older version of the same module.

oscarbenjamin avatar Jul 25 '22 16:07 oscarbenjamin

:white_check_mark:

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

Your release notes are in good order.

Here is what the release notes will look like:

  • other
    • SymPy now depends on the third party multipledispatch module (as installed by pip install multipledispatch) rather the vendoring its own older version of the same module. (#23830 by @oscarbenjamin)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.12.

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. -->

An alternative to #23621. This does not yet fix the import time problem though.

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

Change all usage of `sympy.multipledispatch` to import from and use `multipledispatch`. Add a dependency on the 3rd party Python `multipledispatch` module.

There are some incompatibilities between SymPy's multipledispatch and the PyPI package that need to be resolved before fully deleting the module:

- [X] Change all imports
- [X] Add dependency in setup.py
- [X] Install in CI
- [ ] Make sure all tests pass
- [ ] Resolve any incompatibilities
- [ ] Move any code in the `sympy.multipledispatch` module somewhere else (maybe there could be `sympy/external/multipledispatch`).
- [ ] Delete the `sympy.multipledispatch` module and associated tests
- [ ] Remove all references e.g. in setup.py, LICENSE, etc
- [ ] Improve import time
- [ ] Update docs

#### Other comments

The 3rd party multipledispatch module does not seem to be actively maintained e.g. it hasn't had a release since 4 years ago in 2018. It's not clear that it really needs much in terms of fixes though because it is relatively simple.

We might not want to depend on it as a 3rd party package but if not then at least we should update the vendored version to match the latest PyPI release because it has a number of improvements. At least there has been one fix in SymPy (#22029) that has not been applied to the 3rd party package https://github.com/mrocklin/multipledispatch/issues/118.

Alternatively if it does need more maintenance maybe it would be possible to take over maintenance.

#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers.

or if no release note(s) should be included use:

NO ENTRY

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 -->
* other
  * SymPy now depends on the third party multipledispatch module (as installed by `pip install multipledispatch`) rather the vendoring its own older version of the same module.
<!-- END RELEASE NOTES -->

sympy-bot avatar Jul 25 '22 16:07 sympy-bot

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1 means a speed up and greater than 1 means a slowdown. Green lines beginning with + are slowdowns (the PR is slower then master or master is slower than the previous release). Red lines beginning with - are speedups.

Significantly changed benchmark results (PR vs master)


Significantly changed benchmark results (master vs previous release)


Full benchmark results can be found as artifacts in GitHub Actions (click on checks at the top of the PR).

github-actions[bot] avatar Jul 25 '22 19:07 github-actions[bot]

The 3rd party multipledispatch module does not seem to be actively maintained e.g. it hasn't had a release since 4 years ago in 2018. It's not clear that it really needs much in terms of fixes though because it is relatively simple.

We might not want to depend on it as a 3rd party package but if not then at least we should update the vendored version to match the latest PyPI release because it has a number of improvements. At least there has been one fix in SymPy (https://github.com/sympy/sympy/pull/22029) that has not been applied to the 3rd party package https://github.com/mrocklin/multipledispatch/issues/118.

Alternatively if it does need more maintenance maybe it would be possible to take over maintenance.

That's my only concern here. However, if there is interest in taking over maintenance, perhaps @mrocklin would be open to the idea.

asmeurer avatar Jul 25 '22 19:07 asmeurer

I'd be happy to expand ownership of that repository to a trusted individual. I'd trust @asmeurer if he's volunteering :slightly_smiling_face:

mrocklin avatar Jul 26 '22 00:07 mrocklin

I think @oscarbenjamin is actually the one who's volunteering, but if your trust only extends as far as me I can take that on for now.

asmeurer avatar Jul 26 '22 18:07 asmeurer

I would rather it be an organisational basis like "sympy" would take over maintenance if that is needed. I think that it is generally better to think of maintenance in terms of organisations rather than individuals if possible.

oscarbenjamin avatar Jul 26 '22 20:07 oscarbenjamin

The future in python 3.10 allows structual pattern matching which can outdate some functional capability of multipledispatch.

sylee957 avatar Jan 10 '23 22:01 sylee957