search

alchemyst / Skogestad-Python

Python code for "Multivariable Feedback Control"
111 stars 88 forks source link

Rename .py files to .ipynb fix #358 #360

Closed Bern-Alberts closed 4 years ago

Bern-Alberts commented 4 years ago

I did random spot tests with a few of the newly generated files and found no issues.

review-notebook-app[bot] commented 4 years ago

Check out this pull request on  ReviewNB

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

alchemyst commented 4 years ago

This is a good start, but the goal is to replace the .py files with the .ipynb files instead of just copying, since that would introduce additional maintenance burden. The full fix here also includes updating the tests to run the .ipynb files instead of the .py files.

alchemyst commented 4 years ago

This might be done more easily by using pytest instead of runall.py, see issue #359 .

alchemyst commented 4 years ago

Just so that it links, I'm going to mention the issue #358

Bern-Alberts commented 4 years ago

@alchemyst Fixing the environment variable issues with the link you provided got pytest working and all reproduction notebooks have been tested.

I found issues in 3 notebooks that used the 'execfile' function and have replaced this.

All reproduction notebooks passed the subsequent test.

alchemyst commented 4 years ago

Thanks for this @Bern-Alberts . Unfortunately this has broken the tests on Travis. You can check that you're not breaking stuff by enabling Travis builds on your fork.

alchemyst commented 4 years ago

Hi @Bern-Alberts . Can you please rebase this work to the current version of the repo? There have been some changes to the Python files which now means I can't automatically merge this.

Bern-Alberts commented 4 years ago

@alchemyst Will do

Bern-Alberts commented 4 years ago

@alchemyst I wasn't paying attention when updating "environment.yml". I merged in the changes that were made to some reproduction py files and regenerated their respective notebooks.