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

Add a single unit test to a datascience #84

Open
5 tasks done
arnavs opened this issue May 6, 2021 · 4 comments
Open
5 tasks done

Add a single unit test to a datascience #84

arnavs opened this issue May 6, 2021 · 4 comments
Assignees

Comments

@arnavs
Copy link
Member

arnavs commented May 6, 2021

@mmcky proposed the solutino that the following was added:

:tags: ["remove-cell"]

which should hide both input and output. He said that That way they will execute as part of jupyter-cache but neither input or output will be included for the code-cell that they are contained in for the html. Sounds good enough to me!

Now to verify that this works prior to applying it all over the julia code, we should add in a single unit test in one of the lectures. And we should do it in a very "low risk" place. I suggest going to https://github.com/QuantEcon/lecture-datascience.myst/blob/main/src/applications/visualization_rules.md and adding in something completely at the bottom of the lecture that does

```{code-cell} python
:tags: ["remove-cell"]
print("Unit test running")
assert True, "Failure in unit test"
```

Instead of at the bottom, maybe put it after the text and before something else just so we can see if it messes up spacing. For example change the text in that lecture to be

    ### Exercise 1
    
    Create a draft of the alternative way to organize time and education -- that is, have two subplots (one for each education level) and four groups of points (one for each year).
    ```{code-cell} python
    :tags: ["remove-cell"]
    print("Unit test running")
    assert True, "Failure in unit test"
    ```
    Why do you think they chose to organize the information as they did rather than this way?

OK, after adding that, then you need to verify a few things

  • Leave the assert True as on and generate the HTML. Verify that no funny business happened (e.g. no text shows up, there isn't a weird cell, etc.) and the spacing also wasn't drastically changed
  • Leave the assert True as on and generate the jupyter notebooks. Check if the jupyter notebooks generated with the "Unit test running" and maybe all of that code in a cell.
    • If so, then make sure an issue is posted on the appropriate repo and link to it from this issue. We can launch the notebooks with that small quirk at the end of that notebook until the feature is implemented. People can just ignore the cell until then.
  • I can't remember if we have pdf support yet, but if we do then do the same thing. Hopeefully the cell doesn't show up, but if it does then we should ensure an issue is posted - and not worry about the bug in the pdf output because the stakes are low.
  • Now modify the code to assert False etc. and run things to compille that file locally. Ensure that the error isn't just silent.
  • Make the change to assert False and then commit to a PR. Make sure that the error isn't just silent. This is a "breaking" change and it should be very visible in the CI in one form or another. If it stops the procses of building the full netlify/etc. for the PR that is fine - we just need to make sure the errors aren't silent.
@arnavs
Copy link
Member Author

arnavs commented May 6, 2021

Potentially on a data download for an applications lecture

@arnavs arnavs self-assigned this May 6, 2021
@jlperla jlperla changed the title Unit Test Check Add a single unit test to a datascience May 14, 2021
@aadsouza
Copy link
Contributor

aadsouza commented May 15, 2021

the error message under assert False can be found in the build cache @jlperla @arnavs

@mmcky
Copy link
Contributor

mmcky commented May 17, 2021

thanks for testing this idea our @arnavs @aadsouza -- anything you need my input on? I will use this work in making an adjustment to sphinx-tomyst for julia conversion.

@aadsouza
Copy link
Contributor

@WenxinM as well 😁. Nothing from my end except for the stuff that @jlperla had outlined.

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

No branches or pull requests

4 participants