đ Tutorial study functionality #58
Conversation
bisgaard-itis
added
documentation
There was a problem hiding this comment.
@bisgaard-itis please do not forget to fill the PR issue attributes. e.g.
- assignee
- select project
- add labels
- select milestone
Also, the gh issue links render better if you use them in enumerations, ie.g. issue #1 renders
this is how it renders a flat #1
- but this is how it renders in enum Upgrade v0.2Â #1
| ), f"The version defined in {API_DIR/'config.json'} is not present in {TUTORIAL_CLIENT_COMPATIBILITY_JSON}" | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("tutorial", get_tutorials()) |
There was a problem hiding this comment.
TIP: using ids= argument will display the parametrization much clearer.
e.g. try running it now and see the output vs
@pytest.mark.parametrize("tutorial", get_tutorials(), ids: lambda p: p.name)
def test_...
``There was a problem hiding this comment.
This is a good tip. I was actually trying to figure out how to do this, but didn't find any good tips on google or perplexity
| pytest.skip( | ||
| f"{tutorial.relative_to(DOCS_DIR)} is not compatible with osparc.__version__=={osparc.__version__}. See {TUTORIAL_CLIENT_COMPATIBILITY_JSON.name}" | ||
| ) | ||
| print(f"Running {tutorial.relative_to(DOCS_DIR)}") |
There was a problem hiding this comment.
this print will be unnecessary if you use the parametrization ids above.
@pytest.mark.parametrize("tutorial", get_tutorials(), ids: lambda p: p.relative_to(DOCS_DIR))
def test_...| ), f"Some tutorial notebooks are not present in {TUTORIAL_CLIENT_COMPATIBILITY_JSON}" | ||
|
|
||
| # check that version of this repo is present in compatibility json | ||
| cur_version: str = json.loads((API_DIR / "config.json").read_text())["python"][ |
There was a problem hiding this comment.
STYLE: prioritize good variable names and structure to doc.
e.g. these lines of code do not need extra doc
current_version = json.loads((API_DIR / "config.json").read_text())["python"]["version"]
compatible_versions = set(json.loads(TUTORIAL_CLIENT_COMPATIBILITY_JSON.read_text())["versions"].keys())
assert current_version in compatible_versionsThere was a problem hiding this comment.
Makes sense. Will fix
| @pytest.mark.parametrize("notebook", all_notebooks) | ||
| def test_run_notebooks(tmp_path: Path, notebook: Path, params: dict[str, Any] = {}): | ||
| """Run all notebooks in the documentation""" | ||
| def run_notebook(tmp_path: Path, notebook: Path, params: dict[str, Any] = {}): |
There was a problem hiding this comment.
STYLE: every function not being fixtures or tests in a test/*/test_*.py file are helpers.
- The fixtures are decorated with
pytest.fixture - Tests are named
test_ - (our convention) helpers use protected prefix, i.e.
def _run_notebook(...)
There was a problem hiding this comment.
Makes sense. I will add a _ in front of these.
| @@ -45,3 +46,90 @@ def test_run_notebooks(tmp_path: Path, notebook: Path, params: dict[str, Any] = | |||
| kernel_name="python3", | |||
| parameters=params, | |||
| ) | |||
|
|
|||
|
|
|||
| def get_tutorials(osparc_version: Optional[str] = None) -> List[Path]: | |||
There was a problem hiding this comment.
TIP: I think there is a packaging.version.Version object
There was a problem hiding this comment.
Yes, I know about that one. However, since here I use the input to extract something from a dict (loaded from a json). So I think it is makes sense to simply use the string here. Let me know if you disagree.
| @@ -45,3 +46,90 @@ def test_run_notebooks(tmp_path: Path, notebook: Path, params: dict[str, Any] = | |||
| kernel_name="python3", | |||
| parameters=params, | |||
| ) | |||
|
|
|||
|
|
|||
| def get_tutorials(osparc_version: Optional[str] = None) -> List[Path]: | |||
There was a problem hiding this comment.
Q: Optional or Dict annotations are because we have to be compatible with 3.6>?
There was a problem hiding this comment.
I don't think this has to be compatible with <3.6. Currently I use 3.9 when developing the client. I can replace the Dict by dict, but what about the Optional? Should that be replaced by <> | None?
Sorry, I thought I had already filled in all the PR attributes. I will be more careful next time. |
What do these changes do?
Add a first tutorial for the studies entry.
Related issue/s
How to test