-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PR: Added support for sphinx_math_dollar in help text generation #16586
base: master
Are you sure you want to change the base?
Conversation
Hello @oscargus! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2021-12-19 13:22:18 UTC |
6e4280e
to
de077af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @oscargus, thanks for your contribution! I think this is a very nice addition.
I only have one question for you: should we add this extension as a new Spyder dependency? Otherwise, it'd be hard for people to discover this functionality.
What do you think?
I was also considering adding it as a dependency. It is a quite small package, so from that perspective it may not be a big deal. On the other hand, it seems to primarily be a benefit for SymPy (right now, not sure how many other packages use it, yet), so that is why I didn't add it. Also, it will not break anything, just looks a bit worse compared to what it can. But I'd be happy to add it. I definitely see the benefit of it as many people will probably not realize that there is a way to get rid of those $. (I primarily contribute to SymPy, where there is a single dependency. So I guess I may be a bit biased regarding adding dependencies...) |
194c0f5
to
cf7fd8b
Compare
cf7fd8b
to
7f99f2d
Compare
Finally got around to updating this. Took a bit of time as I'd say that the logic is a bit more correct now since earlier on packages in SKIP_PACKAGES was not correctly handled as modules (as the set subtraction was on strings with version numbers). Although it doesn't really makes any difference, it should be better now. Also, I added a feature so that all packages with The MacOS-app failure seems unrelated and maybe I should have merged against |
Description of Changes
Added support for the
sphinx_math_dollar
plugin that e.g. SymPy uses.https://www.sympy.org/sphinx-math-dollar/
The idea is that math equations can be marked using the LaTeX-style
$...$
or$$...$$
instead of the more tedious Sphinz marking.With:
Without:
This checks if
sphinx_math_dollar
exists and if so adds it to the sphinx configuration (assuming that MathJax is used).Not clear how to add a test for it. It seems like there are no tests for math rendering in the help plugin at all.
If it makes sense, I can add an additional check box for it in the preferences. But I think that if someone has installed sphinx_math_dollar (it is not a mandatory dependency of SymPy), they probably would like to use it by default.
Issue(s) Resolved
Affirmation
By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.
I certify the above statement is true and correct:
oscargus, Oscar Gustafsson