Solution for 'intentional' errors

[natashawatkins]

We need a way to label cells that produce errors, without having the errors count towards coverage (https://lectures.quantecon.org/status.html). Possible we could introduce a skip-test label.

[natashawatkins]

@mmcky thinking about this, I think we need both a skip-test label for errors we want to display, and a no-execute label for code we don't want run (ie. C code), which should also no count towards the testing stats

[mmcky]

@natashawatkins thinking this through over the past through days.

Current

• for now -- we will keep :no-execute: in place for testing and I will add an option to jupinx to enable an ignore on the :no-execute: flag for compilation of the website. (Short Term Fix). This can be enabled by supplying the command line argument jupyter_ignore_no_execute=1 to the sphinx-build command in the Makefile

The negative of this approach is that testing and rendering produce different sets of ipynb notebooks so we can't use a common set for now. This will need to be taken into consideration when setting up the cache pathways.

Future

• it would be better to add :no-execute: to cell metadata and write a nbconvert pre-parser to skip execution.

class MyExecutePreprocessor(nbconvert.preprocessors.ExecutePreprocessor):

    def preprocess_cell(self, cell, resources, cell_index):
        """
        Executes a single code cell. See base.py for details.
        To execute all cells see :meth:`preprocess`.

        Checks cell.metadata for 'execute' key. If set, and maps to False, 
          the cell is not executed.
        """

        if not cell.metadata.get('execute', True):
            # Don't execute this cell in output
            return cell, resources

        return super().preprocess_cell(cell, resources, cell_index)

(as per: https://stackoverflow.com/questions/26494747/simple-way-to-choose-which-cells-to-run-in-ipython-notebook-during-run-all/43584169)

• For the .. jupyter:: directive we can add :allow-error: for cell-blocks that should execute (when compiling the website) but don't execute when running the coverage test suite. In the future we could add error types such as ValueError to catch only specific error types etc. For now it will be just a boolean flag. It is also pretty self explanatory.

Also if there are blocks that are not the same as the default language (i.e. python3) then they should be added as markdown blocks anyway in jupinx conversion.

[mmcky]

This PR QuantEcon/sphinxcontrib-jupyter#129 implements the SHORT TERM solution for this issue. The Makefile for ``quantecon.build.lectures` has also been updated.

[natashawatkins]

So at the moment we won't be displaying errors? Does that mean that we need to manually put them in?

[mmcky]

@natashawatkins not sure what you mean here. The japing fix allows for those :no-execute: statements to be ignored during the compilation of the notebook so they will be in execute blocks rather than markdown blocks when building the lectures. When running tests they will be honoured.

[natashawatkins]

I'm confused - there are two separate cases:

1. Code that intentionally has an error and we want to display the error output.
2. Code that should not be executed - we don't want to display the error. Eg. a partial solution to an exercise, see here: https://lectures.quantecon.org/py/python_advanced_features.html#exercise-2

no-execute should be used in the second case - is that right?

[mmcky]

Ah so there are really 3 cases.

I was working to:

1. Same as your 1 above
2. Show code such as C code in a python lecture.

This 2 case is handed by Jupinx:

Also if there are blocks that are not the same as the default language (i.e. python3) then they should be added as markdown blocks anyway in jupinx conversion.

but there is a third case such as your 2 above. Bummer :) Can you think of any others?

[natashawatkins]

So what should we do in the first case? Will jupinx insert the error output and not count the error towards the testing stats?

And not that I can think of.

[mmcky]

OK lets do the following convention for now:

1. no-execute for anything that should never be executed (partial solution). This will end up as highlighted markdown but coded as a code-block
2. skip-test for anything that should be executed (i.e. purposeful error) but shouldn't be counted in any coverage tests
3. .. code-block:: C should be handled by jupinx already and will be a highlighted markdown block if C is not the default language of the notebook.

@natashawatkins would you mind adding PR's for the new convention and I will work on jupinx today.

[mmcky]

• added ability to globally skip-test and updated quantecon.build.lectures and jupinx (FEAT: add ignore for globally skipping skip-test flag in code-blocks sphinxcontrib-jupyter#131)
• adjust rst files

[natashawatkins]

Is @fkazemian available to work on this maybe? I can check the PRs

[fkazemian]

Hi @natashawatkins I will do it

[fkazemian]

stationary_densities

It's fine

[fkazemian]

Hi @mmcky
I think in here we have to use
skip-test for anything that should be executed (i.e. purposeful error) but shouldn't be counted in any coverage tests

because we want it to be executed, but when I'm compiling this code:

.. code-block:: python3
    :class: skip-test

    f = open('us_cities.txt')
    f.__next__()
    
.. code-block:: none
    :class: skip-test
    
    'new york: 8244910\n'
    
.. code-block:: python3
    :class: skip-test

    f.__next__()

but it's not working, Do I use a right tag?

@fkazemian that example above is not a purposeful error. That is an error because the file us_cities.txt cannot be found (Issue: #91). Please take your time on this. In the vast majority of cases it will replace :no-execute:.

[mmcky]

for skip-test to work you will need to latest version of jupinx (from master). This was only merged yesterday. If you have the latest version please let me know and I will take a closer look.

[natashawatkins]

@fkazemian could you also do this issue in separate PRs for each lecture? thanks

[fkazemian]

excuse me Have you tested the latest version of jupinx locally?
I don't know why mine is like

[mmcky]

hi @fkazemian if you are not using quanecon.build.lectures to compile then it is because the theme has not been applied to the generated output.

[fkazemian]

Hi @mmcky Actually I'm using 'quanecon.build.lectures' and I used git pull origin master

[natashawatkins]

@fkazemian could you please include these comments in separate PRs because it's a bit hard to follow the above comment. Thanks

[fkazemian]

sure
@natashawatkins , I changed something and leave something unchanged. I explain my changes on its PRs and If I leave something unchange and there is no PR for It I'll explain it in separate comments here to review.

[fkazemian]

stationary_densities

• ./rst_files/stationary_densities.rst: :class: no-execute

.. code-block:: python3
    :class: no-execute

    initial_conditions = np.linspace(8, 0, J)



For each :math:`X_0` in this set,

• ./rst_files/stationary_densities.rst: :class: no-execute
  

For the four initial distributions, use the shifted beta distributions

.. code-block:: python3
    :class: no-execute

    ψ_0 = beta(5, 5, scale=0.5, loc=i*2)

[fkazemian]

python_oop

• ./rst_files/python_oop.rst: :class: no-execute
• ./rst_files/python_oop.rst: :class: no-execute
  
  it's fine, because by executing it we get a NameError due to name 'ECDF' is not defined

---

[fkazemian]

sci_libs

• ./rst_files/sci_libs.rst: :class: no-execute
  

• we have to use .. code-block:: C which is fine here, but it also use :class: no-execute, I think we don't want to run it as well, so I leave it unchanged:

Here's a C function that will do the same thing

.. code-block:: c
    :class: no-execute

    double geo_prog(double alpha, int n) {
        double current = 1.0;
        double sum = current;
        int i;
        for (i = 1; i <= n; i++) {
            current = current * alpha;
            sum = sum + current;
        }
        return sum;
    }


If you're not familiar with C, the main thing you should take notice of is the
type definitions

[fkazemian]

numba

• ./rst_files/numba.rst: :class: no-execute
  

• we have to use .. code-block:: C which is fine here, but it also use :class: no-execute, I think we don't want to run it as well, so I leave it unchanged

For example, consider the following C code, which sums the integers from 1 to 10

.. code-block:: c
    :class: no-execute

    #include <stdio.h>

    int main(void) {
        int i;
        int sum = 0;
        for (i = 1; i <= 10; i++) {
            sum = sum + i;
        }
        printf("sum = %d\n", sum);
        return 0;
    }

The variables ``i`` and ``sum`` are explicitly declared to be integers

[fkazemian]

jv

• ./rst_files/jv.rst: :class: no-execute
  

 Set

.. code-block:: python3
    :class: no-execute

    K = 50
    plot_grid_max, plot_grid_size = 1.2, 100
    plot_grid = np.linspace(0, plot_grid_max, plot_grid_size)
    fig, ax = plt.subplots()
    ax.set_xlim(0, plot_grid_max)
    ax.set_ylim(0, plot_grid_max)


By examining the plot, argue that under the optimal policies, the state

[fkazemian]

Hi @natashawatkins , would you please review these PRs and comments? I leave something unchanged which I explained them in comments here(If there were no PR for them) and I also explain my changes as well.
python_advanced_features #99
schelling #103
python_essentials #98
pandas.rst #102
debugging #97
finite_markov #101
python_by_example #100
oop_intro #96
stationary_densities #85 (comment)
python_oop #85 (comment)
sci_libs #85 (comment)
numba #85 (comment)
jv #85 (comment)

[natashawatkins]

@fkazemian I think you've misunderstood the tags - you want to use :skip-test: when the error should be generated and displayed, and :no-execute: when we don't want the error displayed. Could you be go through and fix the PRs accordingly?

[natashawatkins]

@fkazemian could you also please remove all the tags you've added to .. code-block:: none cells - in some instances these cells should be completely removed if they are automatically generated

[mmcky]

@fkazemian could you liaise with @natashawatkins to update your PR's re: this issue. Careful attention to detail is required on this one to figure out the 3 different states for each case.

[mmcky]

this is the last remaining high priority issue to fix I think :)

[fkazemian]

Hi @natashawatkins @mmcky sure

[mmcky]

Closing this. Any unaddressed bugs will come up in the next comparison review.