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

Markdown parsing #547

Closed
wants to merge 4 commits into from
Closed

Markdown parsing #547

wants to merge 4 commits into from

Conversation

bqpd
Copy link
Contributor

@bqpd bqpd commented Mar 5, 2016

This PR allows a markdown file to be run as python code. For example:

import gpkit; exec gpkit.mdmake("gas_hale.md")

will run the code in this example markdown file.

This provides immediate model documentation (including plots) on Github. Check it out!

A secondary advantage is that running pandoc -o gas_hale.pdf gas_hale.md produces this nice pdf

@bqpd
Copy link
Contributor Author

bqpd commented Mar 5, 2016

@whoburg, @galbramc, @pgkirsch curious for your feedback on this. Useful for 16.82?

@bqpd
Copy link
Contributor Author

bqpd commented Mar 5, 2016

Thinking of adding the following as a shell script or makefile: @whoburg, thoughts?

for var in $@; do
    python -c "import gpkit; exec gpkit.mdmake(\"$var\")"
    if [ -e "default.latex"]; then
        pandoc -c "pandoc -o $var.pdf  $var --template default.latex
    else
        pandoc -c "pandoc -o $var.pdf  $var
    done
done

@bqpd
Copy link
Contributor Author

bqpd commented Mar 5, 2016

Also keeping an eye open for ways we could indicate that for the PDF a particular code block should be removed or replaced by a particular .tex file, so that model.md could compile directly to one of the subsections of @pgkirsch's paper, etc.

Currently thinking that if the first line of a python block is #inPDF: hide or #inPDF: replace with vartable.tex then the block should be respectively removed or replaced with \include{vartable.tex} when compiling to PDF.

@bqpd
Copy link
Contributor Author

bqpd commented Mar 5, 2016

Implemented the above in latest commit, updated PDF is here: gas_hale.pdf

In the .md file: skip a code block / replace it with latex.

@bqpd
Copy link
Contributor Author

bqpd commented Mar 5, 2016

I'm digging the idea of starting with code and incrementally replacing it with latex that describes it: the bits of the PDF that are still code are the bits that need more documentation work... @pgkirsch, you're the expert on gpkit papers, does this seem reasonable? (with an overall tex file that imports all the submodel texs, etc.)

@pgkirsch
Copy link
Contributor

pgkirsch commented Mar 6, 2016

hmmm this is an interesting idea, and seems like it could be really helpful for documentation and paper writing.

A couple initial reactions:

  • It feels odd that we are trying to optimize/automate the GPkit paper-writing process when the group only has a couple paper's worth of experience. Isn't this a little premature?
  • I'm worried about adding syntax to GPkit scripts that makes them less easy to understand. One of the nice things about GPkit at the moment is that it's all very lightweight and easy to pickup. It's not like your proposed additions are overly complex, but it's just more things for a new user to learn/understand. I also like the idea of keeping the GPkit interface static for just a little while to let the user base grow before adding loads of bells and whistles. I feel like the flavor of GP changes so often that it might be frustrating for people to have to keep getting up to speed with new interfaces.
  • I'm also worried about making files longer than they need to be, and adding overhead.

That probably sounds really negative, sorry. I think it's cool stuff and really nice for auto-generating documentation, but is there really a demand for it yet when we can't even debug models that don't work/don't have that many models in our model library?

@bqpd
Copy link
Contributor Author

bqpd commented Mar 7, 2016

Good points!

re 1, this experiment is definitely a little premature. :) But Woody and I were looking at the 16.82 repo and saw latex documentation of models which was just copy-pasted comments variables and constraints, so it might be immediately useful.

re 2, One nice thing about this is that it doesn't add any new python syntax.

But, re 3, it does increase the complexity of the model file by incorporating more documentation into it. The hope is that this reduces the overall difficulty of documenting a model (see for instance the picture you posted for the atmospheric model) and the likelihood that documentation and code will slip out of sync.

re your last paragraph, I think we need to start early with good documentation of the models in our library. The atmospheric model's been great for this, with citations and graphs, and this might be a way to normalize that kind of model documentation.

@whoburg
Copy link
Collaborator

whoburg commented Mar 7, 2016

retest this please

@whoburg
Copy link
Collaborator

whoburg commented Mar 7, 2016

Great to see an initial proposal for how to tie together model implementation with documentation / derivation.

A few thoughts:

  • is it possible to put raw latex math in the .md? This is critical -- e.g. someone might want to give the derivation that led up to some constraint in a model -- those derivation equations won't come from Python.
  • probably belongs in gpkit-models, but I think the latex template should \usepackage{fullpage} -- right now many code blocks are running off to the right. An 80 char block should look good.
  • Impressive that this is implemented in 24 lines -- let's keep it clean instead of implementing our own markdown language
  • on that note, not sure the code implementing inPDF: got pushed to the remote
  • how would imported models work, e.g. from gpkit.models.atmosphere import Trposphere? Where would their documentation live and how would one import it if desired?
  • At a high level I like the direction but I still have questions about the implementation. This proposal implements one option: models and documentation in .md files. I'm also wondering about a second option, where models live in .py files and explicitly create .tex output files, which can then be included in a separate latex file describing the model. Not saying this is right -- just want to think about pros / cons.

@@ -26,6 +26,30 @@
SETTINGS_PATH = os_sep.join([os_path_dirname(__file__), "env", "settings"])


def mdparse(filename):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstring

@whoburg
Copy link
Collaborator

whoburg commented Mar 7, 2016

one more comment: it feels a bit awkward to have the python implementation of a particular class (class GasPoweredHALE(Model):, in this case) broken up by text and documentation. E.g. it looks odd to see code indented so far. I imagine this would improve with model composition though, because everything will be split into separate models, with one code block per model.

@bqpd
Copy link
Contributor Author

bqpd commented Mar 7, 2016

  • Yup it's possible to put raw latex math in the .md; it's done in the example a couple of times
  • (moved to gpkit-models)
  • Added a few more lines of code when
  • whoops, pushed up the inPDF flag
  • The easiest way to implement importing would be to give gpkit.models a function that imported the appropriate model with the docstring set to the md file (so that one could do help(Troposphere) to get the documentation)
  • The second option seems quite reasonable, and requires no new lines of code! Note that the .py and .tex.md file written by mdmake are good starts for the two separate files, so this one-file method isn't removing that information. I think the biggest con for me is the concern that model documentation (explaining constants or constraints) is more likely to slip behind or be inconsistent with the actual model if you can't see them both on the same screen.

re your last comment, it does feel a little funny, but I think it helps when reading the markdown. And yeah, it'd happen a lot less with model composition. :)

@acdl-jenkins
Copy link

Can one of the admins verify this patch?

@galbramc
Copy link
Contributor

galbramc commented Mar 8, 2016

test this please

@whoburg
Copy link
Collaborator

whoburg commented Mar 9, 2016

whoburg$ pandoc -o aircraft_design.pdf aircraft_design.md.tex.md
! LaTeX Error: Environment longtable undefined.

See the LaTeX manual or LaTeX Companion for explanation.
Type  H <return>  for immediate help.
 ...

l.7 \begin{longtable}

pandoc: Error producing PDF

@whoburg
Copy link
Collaborator

whoburg commented Mar 9, 2016

I think it would make sense to move mdparse and mdmake to gpkit.tools, and then implement @bqpd's proposed shell script / make file, which can handle the import from gpkit.tools

@bqpd
Copy link
Contributor Author

bqpd commented Mar 9, 2016

Sounds like a good plan!

The pandoc error is because you didn't use the template including longtable

@whoburg
Copy link
Collaborator

whoburg commented Mar 10, 2016

should that template be added to gpkit?

@bqpd
Copy link
Contributor Author

bqpd commented Mar 10, 2016

In gpkit-models? Quite possibly. In gpkit? I don't know...and what about the shell script?

@bqpd
Copy link
Contributor Author

bqpd commented Mar 10, 2016

This commit was merged into #493, closing

@bqpd bqpd closed this Mar 10, 2016
@bqpd bqpd deleted the mdparse branch March 10, 2016 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants