Skip to content
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

Convert doc to nbsphinx #9

Merged
merged 31 commits into from Mar 11, 2019

Conversation

@utensil
Copy link
Member

utensil commented Sep 24, 2018

My method is to automate when possible but do it manually when not.

  • Convert LaTeX to markdown with pandoc: pandoc -f latex -t markdown_github galgebra.tex -o galgebra.md
  • Manually fix obvious math equations conversion problems
  • Fix equation numbering and referencing in MathJax (which I have no clue for now)
  • Convert part of the markdown with python code to notebooks using notedown (tried, but defined new command can't work across cells, possibly add a init_cell?)
  • Run nbval for these python code in notebooks and add these tests to coverage
  • Convert to readthedocs using nbsphinx
  • Publish to readthedocs: https://galgebra.readthedocs.io/en/latest/

Updated TODOs ( Feb 2019) :

  • Add badges
  • Clean up doc/
  • Fix obvious display problems in the current readthedocs documentation ( #9 (comment) )
  • Update installation instructions and remove the old ones
  • Add migration guide from brombo/galgebra
  • Rewrite the intro section of sphinx doc
  • A feature diff between this fork and the upstream
  • Add links to generated doc in README (done in the form of badge, but will add one more link after the crash course)
  • A "quick start" or "crash course" as discussed in #9 (comment)

Tasks left for later PRs:

  • Pick a better getting started example
  • Improve Section Features in README
  • Write a feature-complete tutorial like clifford's and ganja.js's
  • Break into a few small markdown files
  • Complete proofreading of the whole document and fix formula issues
  • Fix outdated description in document
  • Refer to historical document for users encountering the outdated description in current document
  • Improve API document
  • Convert the python code and pdf output referenced in the tex doc to notebooks
  • Further fix cross references
  • Add description and attributions for the included materials in Section Resources
@utensil utensil force-pushed the nbsphinx branch from cbcabde to bde6560 Sep 24, 2018
@codecov

This comment has been minimized.

Copy link

codecov bot commented Sep 24, 2018

Codecov Report

Merging #9 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #9   +/-   ##
=======================================
  Coverage   48.89%   48.89%           
=======================================
  Files           8        8           
  Lines        4947     4947           
=======================================
  Hits         2419     2419           
  Misses       2528     2528
Impacted Files Coverage Δ
galgebra/printer.py 41.22% <ø> (ø) ⬆️
galgebra/lt.py 22.22% <ø> (ø) ⬆️
galgebra/metric.py 57.44% <ø> (ø) ⬆️
galgebra/mv.py 50% <ø> (ø) ⬆️
galgebra/__init__.py 100% <ø> (ø) ⬆️
galgebra/utils.py 62.5% <ø> (ø) ⬆️
galgebra/ga.py 64.81% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b17b6bb...3060af0. Read the comment docs.

@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Sep 25, 2018

Merging #9 into master will increase coverage by 8.63%.
galgebra/ga.py 64.81% <0%> (+2.05%)
galgebra/metric.py 56.54% <0%> (+2.07%)
galgebra/printer.py 40.8% <0%> (+10.5%)
galgebra/lt.py 22.48% <0%> (+11.4%)
galgebra/mv.py 49.8% <0%> (+12.72%)

✌️

@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Sep 28, 2018

The doc is now published to https://galgebra.readthedocs.io/en/latest/index.html using this branch. @arsenovic @hugohadfield

image

image

image

# Catch-all target: route all unknown targets to Sphinx using the new
# "make mode" option. $(O) is meant as a shortcut for $(SPHINXOPTS).
%: Makefile
notedown galgebra.md > galgebra_guide.ipynb

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

I convert the .tex to a .md, fix it manually(a lot of work), and convert the .md to .ipynb using notedown and then use nbsphinx to convert the .ipynb to build the sphinx which is based on .rst.

My major concern for this method is that we have to maintain .md and generate .ipynb manually and commit it to git (readthedocs doesn't run this makefile). And it deviate from the original tex due to some MathJax limitations(especially of equation referencing).

You may wonder, why go through all these trouble? Because none of the direct method works:

  • The original doc in tex form was written in a not so rigorous way, most of the equations are not surrounded in $ or $$, so pandoc didn't convert it properly to a .rst with math block.
  • .md, even with the help of recommonmark, is not properly handled by sphinx, especially the formulas, but after notedown and nbsphinx, it's much easier to fix

This comment has been minimized.

Copy link
@hugohadfield

hugohadfield Feb 13, 2019

Collaborator

This sounds super annoying, the docs look amazing though

This comment has been minimized.

Copy link
@utensil

utensil Feb 14, 2019

Author Member

Yeah, that's why I gave myself some time to step back and see if there's a better way. So far it's still the best working way.

@@ -0,0 +1,14 @@
/* https://rackerlabs.github.io/docs-rackspace/tools/rtd-tables.html */
/* override table width restrictions */

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

This is to fix the table. Without this, the long sentences in tables will overflow with horizontal scroll instead of wrap properly.

{{ fullname }}
{{ underline }}

.. automodule:: {{fullname}}

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

This is to customize autosummary to render modules, or it will be in a bad shape for submodules.

doc/conf.py Outdated
# The short X.Y version
version = ''
# The full version, including alpha/beta/rc tags
release = '0.4.1.1'

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

Should use a constant from setup.py or somewhere common. I see it's also redundant in clifford.

This comment has been minimized.

Copy link
@hugohadfield

hugohadfield Feb 13, 2019

Collaborator

Yeah this is annoying for both libraries, maybe we can come up with a nice solution, probably not a big problem unless we start doing a lot of releases :)

This comment has been minimized.

Copy link
@utensil

utensil Feb 14, 2019

Author Member

Yeah, it's not a big deal. We can just keep in mind to update both of them for every release for the time being.

import galgebra
import nbsphinx
# nbsphinx_execute = 'always'
nbsphinx_execute = 'never'

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

I need to disable the execution of notebooks because the python code in the original tex still requires a lot of fix.

doc/conf.py Outdated
"special-members",
"undoc-members",
"private-members",
"show-inheritance",

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

Currently I expose everything in the API doc to make spotting missing docs more easily.

@@ -0,0 +1,2009 @@
This document describes the implementation, installation and use of a geometric algebra module written in python that utilizes the *sympy* symbolic algebra library. The python module ga has been developed for coordinate free calculations using the operations (geometric, outer, and inner products etc.) of geometric algebra. The operations can be defined using a completely arbitrary metric defined by the inner products of a set of arbitrary vectors or the metric can be restricted to enforce orthogonality and signature constraints on the set of vectors. Additionally, a metric that is a function of a coordinate set can be defined so that a geometric algebra over a manifold can be implemented. Geometric algebras over submanifolds of the base manifold are also supported as well as linear multivector differential operators and linear transformations. In addition the module includes the geometric, outer (curl) and inner (div) derivatives. The module requires the *sympy* module and the numpy module for numerical linear algebra calculations. For LaTeX output a LaTeX distribution and pdf viewer must be installed. If the user is interested in using geometric algebra for strictly numerical purposes I would recommend using the *glucat* C++ templates which have a python wrapper for python users (<http://glucat.sourceforge.net/>).

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

I removed the photos because they are not properly sized.

|:-|:-|
| linux | Comes with all versions of linux |
| windows | To install python on windows go to https://www.python.org/downloads/windows/ and install version appropriate for you version of windows. If you wish a more complete/advanced installation go to https://code.google.com/p/pythonxy/. |
| mac | Basic version comes with OSX. For better in-stallation go to http://docs.python-guide.org/en/ latest/starting/install/osx/. |

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

Pandoc is not good at converting tables and especially the multiline ones. All tables are fixed manually.


<script type="text/x-mathjax-config">
MathJax.Hub.Config({TeX: { equationNumbers: { autoNumber: "AMS" } }});
</script>

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

I need to enable equation auto numbering for the references to work. This script will not be executed if I feed this .md directly to sphinx.

MathJax.Hub.Config({TeX: { equationNumbers: { autoNumber: "AMS" } }});
</script>

$$\newcommand{\bm}[1]{\boldsymbol{#1}}

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

The original tex used a lot of custom commands. I can't just expand them, because they make the tex source more simple and expressive. But in order to make all these custom commands to work, please note that:

  • it won't work if fed to sphinix directly.
  • if it's converted to ipynb, it won't work in a Jupyter Notebooks for the last parts of the document, because python codes separated the markdown so the environment is not passed on, but it works after converted through nbsphinx, because underlying they are in the same .rst
dirac_eq.Fmt(3,r'%\text{Dirac Equation\;\;}\nabla \bm{\psi}'+\
r' I \sigma_{z}-e\bm{A}\bm{\psi}-m\bm{\psi}\gamma_{t} = 0')
xpdf(paper='landscape',prog=True)
# FIXME terms of \psi^{ty} are not grouped together

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

All codes in doc/python are somehow not working in a non-trivial-to-fix way, but most of the prints are correct. All incorrect places are compared with my eyes and marked with FIXME. Still working on all the fixes.

sphinx_rtd_theme
recommonmark
sphinx-markdown-tables
notedown

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

I need the last three more dependencies than clifford.

This comment has been minimized.

Copy link
@hugohadfield

hugohadfield Feb 13, 2019

Collaborator

These all seem reasonable



def isstr(s):

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

In order to make string type test work under both python 2 and 3, I need this small util if I don't want to introduce six.

This comment has been minimized.

Copy link
@hugohadfield

hugohadfield Feb 13, 2019

Collaborator

Yeah the Python 2 to 3 string conversion is an absolute nightmare, if we get python3 support working well enough we can probably just ditch python2 support if it turns out to be an annoying problem

This comment has been minimized.

Copy link
@utensil

utensil Feb 14, 2019

Author Member

So far the Python 2 support is not a big problem per se, with the help of CI and tools/libraries we can maintain good support for 2.7 & 3. Also I hope the releases from pygae won't make users having to choose between 2.7 and 3, especially we are at such an early stage of divergence.

Here's just some notes about trying to keep the dependencies minimal, so I prefered an utility function instead of introducing a dependency ( even it's a quite mature and ubiquitous one ).

$\\DeclareMathOperator{\Tr}{Tr}
\\DeclareMathOperator{\Adj}{Adj}
"""
$\\DeclareMathOperator{\\Tr}{Tr}

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

I was wrong to add the r prefix(broke xpdf printing), I just need to fix the \T and \A without double backslashes.

@@ -59,6 +58,7 @@
print_replace_new = None

SYS_CMD = {'linux2': {'rm': 'rm', 'evince': 'evince', 'null': ' > /dev/null', '&': '&'},
'linux': {'rm': 'rm', 'evince': 'evince', 'null': ' > /dev/null', '&': '&'},

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

Without this line, the pdflatex can't be called under Linux.

def com(A, B):
return ga.Ga.com(A, B)
raise ImportError(
"""mv.com is removed, please use ga.Ga.com(A, B) instead.""")

This comment has been minimized.

Copy link
@utensil

utensil Sep 28, 2018

Author Member

Deprecate mv.com.

@utensil utensil force-pushed the nbsphinx branch from 7336e66 to 0c2a20c Sep 28, 2018
@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Sep 28, 2018

There're some strange difference between local build and readthedocs build:

Local:

image

readthedocs:

image

which I have no clue. The .md and .ipynb are fine.

print('\\bm{a|(b^c^d)} =', a | (b ^ c ^ d))
# FIXME this should print 0, got blank
print('\\bm{a|(b^c)+c|(a^b)+b|(c^a)} =',
(a | (b ^ c)) + (c | (a ^ b)) + (b | (c ^ a)))

This comment has been minimized.

Copy link
@utensil

utensil Oct 1, 2018

Author Member

Finally found the reason:

When added together, (a | (b ^ c)) + (c | (a ^ b)) + (b | (c ^ a))) is initially (a.b)c−(b.c)a−((a.b)c−(b.c)a), so it missed the first branch in GaLatexPrinter._print_Mv:

    def _print_Mv(self, expr):
        if expr.obj == S(0):   # which equals False at this stage
            return('0 \n')
        else:
            return expr.Mv_latex_str()

But in Mv.Mv_latex_str(), some code modified expr.obj:

        self.obj = expand(self.obj)
        self.characterise_Mv()
        self.obj = metric.Simp.apply(self.obj)

After the lines above the self.obj now equals S(0).

So a simple yet hacky fix is to add the following after the code above:

        if self.obj == S(0):
            return ' 0 '

But the real fix is to move the simplification-related logic out of the printer method.

The reason was found because I observed:

image

which indicated side effect in printing which is caused by:

image

which is caused by:

image

@utensil utensil force-pushed the nbsphinx branch 2 times, most recently from d9357a5 to d8ca654 Oct 2, 2018
This was referenced Feb 13, 2019
@arsenovic

This comment has been minimized.

Copy link
Member

arsenovic commented Feb 21, 2019

good job with all your work on this.

my only comment is that you might consider adding a short 'crash course' or a 'quick intro' eventually. most people have short attention.

@arsenovic

This comment has been minimized.

Copy link
Member

arsenovic commented Feb 21, 2019

dont forget to add link to the docs in the readme.

@pygae pygae deleted a comment Mar 7, 2019
@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Mar 7, 2019

Comments from codacy-bot for markdown files are super annoying, disabled them on Codacy:

image

@pygae pygae deleted a comment Mar 7, 2019
@pygae pygae deleted a comment Mar 7, 2019
@pygae pygae deleted a comment Mar 7, 2019
@pygae pygae deleted a comment Mar 7, 2019
@pygae pygae deleted a comment Mar 7, 2019
@pygae pygae deleted a comment Mar 7, 2019
@pygae pygae deleted a comment Mar 7, 2019
@utensil utensil force-pushed the nbsphinx branch from 8952e1a to 18f93db Mar 7, 2019
@utensil utensil force-pushed the nbsphinx branch from 18f93db to 2c5af08 Mar 7, 2019
@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Mar 7, 2019

@arsenovic @hugohadfield @waldyrious

Now most of tasks are completed for a first iteration(see the main thread at the top, I kept updating the task, 16 of them has been completed), I propose to merge this PR as is and leave the following tasks to later PRs:

  • Pick a better getting started example
  • Improve Section Features in README
  • Write a feature-complete tutorial like clifford's
  • Break into a few small markdown files
  • Complete proofreading of the whole document and fix formula issues
  • Fix outdated description in document
  • Refer to historical document for users encountering the outdated description in current document
  • Improve API document
  • Convert the python code and pdf output referenced in the tex doc to notebooks
  • Further fix cross references
  • Add description and attributions for the included materials in Section Resources

Comments welcome 😄

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@waldyrious

This comment has been minimized.

Copy link
Contributor

waldyrious commented Mar 7, 2019

There is a one-sentence description in the new README:

Symbolic Geometric Algebra/Calculus package for SymPy.

Is that ok? Or is it distracted by the badges so you missed it?

You're right, that is OK, and I did miss it due to the badges. Perhaps adding the sentence below the badges would make it more visible:

screenshot 2019-03-07 at 20 41 29

...but to be honest, now that you pointed out that the text was there, I can't unsee it anymore, and the current version actually looks good (especially in readthedocs, which adds more padding around the paragraph. So maybe it's not worth bikeshedding over that detail :)

@waldyrious

This comment has been minimized.

Copy link
Contributor

waldyrious commented Mar 7, 2019

do you have any advices on Features or Getting Started? I'm not satisfied with them too.

I think it's a good call to make improvements in further PRs. As a mostly curious layman with negligible mathematical background, I'll be happy to contribute in the development of approachable introductory material :)

@arsenovic

This comment has been minimized.

Copy link
Member

arsenovic commented Mar 11, 2019

yes i think what you have is good. can be improved upon as time allows.

@hugohadfield

This comment has been minimized.

Copy link
Collaborator

hugohadfield commented Mar 11, 2019

Looks really good :)

@utensil utensil merged commit 7ff4a42 into master Mar 11, 2019
7 checks passed
7 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Travis CI - Pull Request Build Passed
Details
codebeat no reportable quality changes
Details
codecov/patch Coverage not affected when comparing a962ddc...a846036
Details
codecov/project 49.14% (+0.24%) compared to a962ddc
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@delete-merged-branch delete-merged-branch bot deleted the nbsphinx branch Mar 11, 2019
@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Mar 11, 2019

image

The failing doc build is simply because I pointed to branch nbsphinx on readthedocs.io, will rectify in a sec.

EDIT: fixed.

@utensil utensil added this to the 0.4.4 milestone Apr 3, 2019
@utensil utensil mentioned this pull request Nov 29, 2019
0 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.