-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Removes Symengine backend from sympy.physics.vector/mechanics/_biomechanics. #25617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Hi, I am the SymPy bot. 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:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.13. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
The changes look good, but two comments: The imports should be using the full imports and not just import from |
Let's make sure this gets flagged as backwards incompatible in the release notes. |
Sorry I just missed this comment when merging.
I think that because mechanics is a "leaf node" in the import graph then it is okay to just import things in the way that users would. The situation where it is important to use fully qualified imports is really for anything that would be imported during the process of
Yes, probably that is not needed... |
Fully qualified imports in library code is something that could be enforced with a linter. Although I've never really been that convinced that it's that important. There's a misconception that |
It wouldn't be important if SymPy was not full of circular imports. |
I agree, and that's my point. It's cargo culted everywhere in the codebase. For instance, most test files use this style. But circular imports are impossible in a test file. Nothing does (or should) import something from a test file, even another test file. I guess you could argue it's more readable, especially when a large number of the imports aren't in |
References to other Issues or PRs
Fixes #25401
Brief description of what is fixed or changed
Symengine was added as a backend for sympy.physics.mechanics and related packages and modules to improve performance. Instead it created a maintenance burden because the developers of these SymPy packages do not develop Symengine and many additions improvements to the SymPy packages require duplicating functionality in Symengine. This PR removes support for the Symengine backend in a backwards compatible fashion. If a user was using this, results will not change but their code execution may become slower.
Other comments
Release Notes