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.

Sign up for GitHub

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

Deprecate kedro test and kedro lint #1873

Merged
merged 5 commits into from
Sep 29, 2022
Merged

Deprecate kedro test and kedro lint #1873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d48cdde
Deprecating `kedro test` and `kedro lint`
noklam Sep 26, 2022
e787b74
Deprecate commands
noklam Sep 26, 2022
8aa32d6
Make kedro looks prettier
noklam Sep 27, 2022
ac8b294
Merge branch 'main' into feat/deprecate-kedro-test-lint
noklam Sep 28, 2022
bd9cc62
Update Linting
noklam Sep 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions RELEASE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@

## Bug fixes and other changes

* Fixed `format` in `save_args` for `SparkHiveDataSet`, previously it didn't allow you to save it as delta format.
## Upcoming deprecations for Kedro 0.19.0

## Breaking changes to the API
* `kedro test` and `kedro lint` will be deprecated.
* Fixed `format` in `save_args` for `SparkHiveDataSet`, previously it didn't allow you to save it as delta format.

# Release 0.18.3

Expand Down
26 changes: 21 additions & 5 deletions kedro/framework/cli/project.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,14 @@ def project_group(): # pragma: no cover
@forward_command(project_group, forward_help=True)
@click.pass_obj # this will pass the metadata as first argument
def test(metadata: ProjectMetadata, args, **kwargs): # pylint: disable=unused-argument
"""Run the test suite."""
"""Run the test suite. (DEPRECATED)"""
deprecation_message = (
"DeprecationWarning: Command 'kedro test' is deprecated and "
"will not be available from Kedro 0.19.0. "
"Use the command 'pytest' instead. "
Copy link
Member

Choose a reason for hiding this comment

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

Just to be clear, this means we are not wrapping pytest in a Kedro command, but we're still supporting pytest, right? Since, if we were just going to say "testing is up to you now," we shouldn't call out pytest in the message.

I think it's fine as-is, if I've understood correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we will still have some tests in starters? This isn't too important though, as this deprecating message is for 0.18.x, we will still support pytest until 0.19.0, so this message is just asking people to use pytest instead of kedro test.
#1879 is a more detailed docs for how to use pytest.

Copy link
Contributor

Choose a reason for hiding this comment

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

Determining whether or not tests will still be in starters is part of an open issue (#1849).

Copy link
Contributor

Choose a reason for hiding this comment

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

I personally wouldn't mention pytest as an alternative in this message, I think this also would keep things consistent with the linting message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am gonna leave it as is. I don't think it matters too much and I am fine with either option.

I checked in 0.17.x we did similar thing for kedro install

kedro/kedro/framework/cli/project.py

Lines 175 to 180 in 80c0d3a

deprecation_message = (
"DeprecationWarning: Command `kedro install` will be deprecated in Kedro 0.18.0. "
"In the future use `pip install -r src/requirements.txt` instead. "
"If you were running `kedro install` with the `--build-reqs` flag, "
"we recommend running `kedro build-reqs` followed by `pip install -r src/requirements.txt`"
)

)
click.secho(deprecation_message, fg="red")

try:
_check_module_importable("pytest")
except KedroCliError as exc:
Expand All @@ -90,7 +97,13 @@ def test(metadata: ProjectMetadata, args, **kwargs): # pylint: disable=unused-a
def lint(
metadata: ProjectMetadata, files, check_only, **kwargs
): # pylint: disable=unused-argument
"""Run flake8, isort and black."""
"""Run flake8, isort and black. (DEPRECATED)"""
deprecation_message = (
"DeprecationWarning: Command 'kedro lint' is deprecated and "
"will not be available from Kedro 0.19.0."
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to point out that users should call linters directly? Do we need to specify which ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am less sure about this one, maybe it will become more clear when #1620 is done. I am fine we either wait until #1620 is done or just merge this one and add some links when the docs PRs are done.

)
click.secho(deprecation_message, fg="red")

source_path = metadata.source_dir
package_name = metadata.package_name
files = files or (str(source_path / "tests"), str(source_path / package_name))
Expand Down Expand Up @@ -171,13 +184,15 @@ def package(metadata: ProjectMetadata):
@click.pass_obj # this will pass the metadata as first argument
def build_docs(metadata: ProjectMetadata, open_docs):
"""Build the project documentation. (DEPRECATED)"""
source_path = metadata.source_dir
package_name = metadata.package_name
deprecation_message = (
"DeprecationWarning: Command 'kedro build-docs' is deprecated and "
"will not be available from Kedro 0.19.0."
)
click.secho(deprecation_message, fg="red")

source_path = metadata.source_dir
package_name = metadata.package_name

python_call("pip", ["install", str(source_path / "[docs]")])
python_call("pip", ["install", "-r", str(source_path / "requirements.txt")])
python_call("ipykernel", ["install", "--user", f"--name={package_name}"])
Expand Down Expand Up @@ -262,12 +277,13 @@ def activate_nbstripout(
metadata: ProjectMetadata, **kwargs
): # pylint: disable=unused-argument
"""Install the nbstripout git hook to automatically clean notebooks. (DEPRECATED)"""
source_path = metadata.source_dir
deprecation_message = (
"DeprecationWarning: Command 'kedro activate-nbstripout' is deprecated and "
"will not be available from Kedro 0.19.0."
)
click.secho(deprecation_message, fg="red")

source_path = metadata.source_dir
click.secho(
(
"Notebook output cells will be automatically cleared before committing"
Expand Down