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 --no-kernel option #382

Merged
merged 4 commits into from
Jan 11, 2024
Merged

Add --no-kernel option #382

merged 4 commits into from
Jan 11, 2024

Conversation

arneso-ssb
Copy link
Member

@arneso-ssb arneso-ssb commented Jan 10, 2024

When using ssb-project with templates not using Jupyter Notebooks, there is no need to create a kernel and add the ipykernel dependency to the template that is to be instantiated. The --no-kernel option makes it possible to use ssb-project with "plain" templates.

Some comments to the implementation and the three commits:

  • The first commit is just to get the nox tests to run cleanly. Updated dependencies to get rid of the safety errors and cleaned up som mypy things. The packages that does not have type hints can be configured in pyproject.toml as described here. This is better than having lots of ignore comments in the code.
  • The second commit contains the main implementation of the --no-kernel option, and a refactoring of the build_project- function to decrease the function's complexity (flagged by flake8 test). Extracted the validate_and_fix_git_config-part to a separate function.
  • The third commit contains fixes for errors flagged by the typeguard nox test. I added a test to test the type and value of the optional parameters when they are not specified on the command line. The type was wrong (typer.models.OptionInfo instead of bool or str) and this is issue 106 on the typer library. The solution is to use the recommended Annotated syntax as described in the typer documentation. I changed the code to use the Annotated syntax for all typer.Option parameters. And added some more tests.

Have a look at it and see what you think, @mallport, @mmwinther and @Andilun.

After the PR I noted that there are problems with coverage reporting in the test.yml GitHub action, and that is has been there for a while. Tried to fix it, but found no quick solution.

Update: Solved the coverage-problem by setting relative_files = true in the [tool.coverage.run] section in pyproject.toml.

- Refactor build_project to reduce complexity (make flake8 tests pass)
@arneso-ssb arneso-ssb added the enhancement New feature or request label Jan 10, 2024
- Solves wrong type for optional parameters
  fastapi/typer#106
- Add testcases for --no-kernel option
@arneso-ssb arneso-ssb force-pushed the option-no-kernel branch 2 times, most recently from 98857ee to 6048072 Compare January 10, 2024 18:34
@mallport
Copy link
Contributor

Very cool! We appreciate the pull request, nice feature and very well described in the PR description.

@arneso-ssb arneso-ssb merged commit 0290ffb into main Jan 11, 2024
15 of 16 checks passed
@arneso-ssb arneso-ssb deleted the option-no-kernel branch January 11, 2024 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants