Skip to content
This repository has been archived by the owner on Aug 13, 2020. It is now read-only.

example directory restructured #100

Merged
merged 10 commits into from
Dec 6, 2016
Merged

example directory restructured #100

merged 10 commits into from
Dec 6, 2016

Conversation

aeroaks
Copy link
Member

@aeroaks aeroaks commented Nov 14, 2016

Regarding #95

  • Converting the .py scripts in examples to Jupyter notebooks
  • Remove the old examples/examples-notebook directory and the .py scripts in examples
  • Make sure these work from mybinder by adding a command to install PyFME

@codecov-io
Copy link

codecov-io commented Nov 14, 2016

Current coverage is 87.25% (diff: 100%)

Merging #100 into master will not change coverage

@@             master       #100   diff @@
==========================================
  Files            28         28          
  Lines          1914       1914          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1670       1670          
  Misses          244        244          
  Partials          0          0          

Powered by Codecov. Last update 5fe3d5a...435bc10

@aqreed
Copy link
Member

aqreed commented Nov 14, 2016

@aeroaks I would like to keep the .py examples files, as they will be needed to simulate and tests new developments. Also, not everyone uses notebooks (I don't, for example), so what I propose is to keep the .ipynb files in the old directory.

@astrojuanlu
Copy link
Member

@aqreed We agreed the other day not to duplicate the examples - it's true that we need tests, but those should be unit tests. Keeping two versions of the examples is a mess, I'm -1 on keeping the .py files.

@aeroaks
Copy link
Member Author

aeroaks commented Nov 14, 2016

@aqreed I have not used it but I think that there is a way in which you can convert notebook file to scripts.
https://nbconvert.readthedocs.io/en/latest/

@aqreed
Copy link
Member

aqreed commented Nov 14, 2016

@Juanlu001 well I do think py examples and nb examples should be two separate entities, Jupyter notebooks are a good thing but we should not impose its use neither to visitors or to developers.

@miqlar
Copy link
Contributor

miqlar commented Nov 14, 2016

I am +1 to having both notebooks and .py's.

Notebooks are nice but annoying for fast checks and copypastes.

@astrojuanlu
Copy link
Member

Pending review:

  • Check links in index.ipynb
  • Check that the notebooks match the scripts (perhaps using jupyter nbconvert Example.ipynb --to python)
  • Check that the notebooks contain all output and plots

Who wants to review the pull request based on these bullet points? You can check out the changes using these commands:

git checkout -b aeroaks-master master
git pull git://github.com/aeroaks/PyFME.git master

@aqreed
Copy link
Member

aqreed commented Dec 1, 2016

When doing jupyter nbconvert Example.ipynb --to python this line is added that crashes when executing the script: get_ipython().magic('matplotlib inline')...after deleting it the script runs fine.

@aeroaks I will send you a pull request with the modifications get all the plots in the examples 006 and 007 (my bad, I forgot to uncomment some of them in the previous PR). Also, example 005 was left executed too so I cleaned it up.

@astrojuanlu
Copy link
Member

astrojuanlu commented Dec 2, 2016

When doing jupyter nbconvert Example.ipynb --to python this line is added that crashes when executing the script: get_ipython().magic('matplotlib inline')...after deleting it the script runs fine.

You have to run your script with ipython:

$ ipython example.py

Then get_ipython will be already in global context.

(Source: http://stackoverflow.com/a/32539282/554319)

@astrojuanlu
Copy link
Member

astrojuanlu commented Dec 2, 2016

By the way, it seems like the conversion template con be modified, in case it's interesting:

http://nbconvert.readthedocs.io/en/latest/customizing.html

This is the default one:

https://github.com/jupyter/nbconvert/blob/master/nbconvert/templates/python.tpl

@AlexS12 AlexS12 merged commit f2a8c2f into AeroPython:master Dec 6, 2016
@AlexS12
Copy link
Member

AlexS12 commented Dec 6, 2016

I have merged the branch so now can have both options at the same time. Nevertheless, as I see that @aqreed seems has more changes to do, we can discuss them in a new pull request

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants