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

Docs update and spellchecking #156

Merged
merged 34 commits into from
Aug 4, 2022
Merged

Docs update and spellchecking #156

merged 34 commits into from
Aug 4, 2022

Conversation

Lasica
Copy link
Contributor

@Lasica Lasica commented Jul 22, 2022

Description

Updating the documentation with extra info about difficulties during the setup with starter kedro and about contributing/local testing. Also adds spellcheck github action,

Resolves #155
Resolves #162
Resolves #119
Resolves #132

PR Checklist

@Lasica
Copy link
Contributor Author

Lasica commented Jul 22, 2022

Question is whether to use the same spellcheck for github action and for pre-commit. There is only one issue as it has external dependencies - it uses aspell or hunspell engines with dictionaries for spellcheck. There is no support in pre-commit for pyspelling tool, there are other spell checkers (codespell, spell-check and typos).

@@ -54,6 +54,17 @@ Change directory to the project generated in /home/mario/kedro/kubeflow-plugin-d
A best-practice setup includes initialising git and creating a virtual environment before running `kedro install` to install project-specific dependencies. Refer to the Kedro documentation: https://kedro.readthedocs.io/
```

There are some adjustments that need to be made to run starter Kedro project on Kubeflow. We need to replace all dots in names of catalog with other characters as Kubeflow does not accept dots in names. You can do it by using `sed` in starter root directory:
Copy link
Member

@em-pe em-pe Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true, currently end 2 end tests are passing with catalog entries having dots. What I would mention instead is that the plugin was not yet adjusted to work with kedro namespaces so using them may cause issues.

The problem with namespaces and dots happens when we're using mounted volume and save artifacts as kubeflow outputs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe the wording needs to be improved? By default the basic example does use kubeflow outputs, so the problem occurs. In e2e we swapped catalog file config from starter to use other types of storage such as gcp bucket.

.github/workflows/spellcheck.yml Outdated Show resolved Hide resolved
.github/workflows/spellcheck.yml Outdated Show resolved Hide resolved
docs/source/03_getting_started/01_quickstart.md Outdated Show resolved Hide resolved
docs/source/04_contributing/02_local_testing.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Lasica Lasica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned issues have been resolved.

docs/source/03_getting_started/01_quickstart.md Outdated Show resolved Hide resolved
docs/source/03_getting_started/01_quickstart.md Outdated Show resolved Hide resolved
docs/source/04_contributing/01_guideline.md Outdated Show resolved Hide resolved
docs/source/04_contributing/02_local_testing.md Outdated Show resolved Hide resolved
@@ -54,6 +54,17 @@ Change directory to the project generated in /home/mario/kedro/kubeflow-plugin-d
A best-practice setup includes initialising git and creating a virtual environment before running `kedro install` to install project-specific dependencies. Refer to the Kedro documentation: https://kedro.readthedocs.io/
```

There are some adjustments that need to be made to run starter Kedro project on Kubeflow. We need to replace all dots in names of catalog with other characters as Kubeflow does not accept dots in names. You can do it by using `sed` in starter root directory:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe the wording needs to be improved? By default the basic example does use kubeflow outputs, so the problem occurs. In e2e we swapped catalog file config from starter to use other types of storage such as gcp bucket.

docs/source/03_getting_started/01_quickstart.md Outdated Show resolved Hide resolved
docs/source/03_getting_started/01_quickstart.md Outdated Show resolved Hide resolved
.github/workflows/spellcheck.yml Outdated Show resolved Hide resolved
.github/workflows/spellcheck.yml Outdated Show resolved Hide resolved
@Lasica Lasica changed the title [WIP] Docs update and spellchecking Docs update and spellchecking Jul 27, 2022
Lasica added 17 commits July 27, 2022 14:47
The new image uses the same tool and is more actively maintained
Recommonmark is no longer supported and points to myst as replacement:
https://recommonmark.readthedocs.io/en/latest/
… setup

Moved local testing as enriched text admonitions to guide with tips about local cluster testing.
As per @mwiewior's suggestion to limit the runs
@Lasica
Copy link
Contributor Author

Lasica commented Jul 29, 2022

Now also resolves #162

It enables the feature of using variables in documentation.

Resolves #136
Spellcheck now also enabled for README and CONTRIBUTING. Fixed spelling mistakes
For code fences a workaround needs to be done, as jinja is ignored inside code fences
Copy link

@mwiewior mwiewior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add kedro-mlflow supported version info

docs/source/03_getting_started/05_authentication.md Outdated Show resolved Hide resolved
docs/spellcheck_exceptions.txt Show resolved Hide resolved
@Lasica Lasica merged commit 64ecb1c into develop Aug 4, 2022
@Lasica Lasica deleted the docs-update branch August 4, 2022 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants