-
Notifications
You must be signed in to change notification settings - Fork 926
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
Should kedro test
and kedro lint
be deprecated?
#1766
Comments
The original research done on this topic: #1293 |
ProposalThe CLI commands
A summary of the key arguments for deprecation:
A summary of the key arguments against deprecation:
My view is also that the commands should be deprecated. Both commands are wrappers for tools with many alternatives. I believe that if a user knows the value of testing and linting code, they either:
In these cases, the commands offer limited to no utility. PlanTo mitigate the disadvantages discussed in the arguments against deprecation:
In #1293, step three of this plan is somewhat controversial, since it represents an up-front and ongoing investment. One option is to create this plugin as a community-led project. In any case, further discussion is warranted for this step. |
This summary makes sense to me, but I'm missing what the concrete the alternatives would be for users currently using |
There are two concrete alternatives: 1. Users Install Testing and Linting Tools IndependentlyUsers install and configure the tools that For isort and black, users can create and enter settings into a pyproject.toml file. For flake8, they need to do the same for a setup.cfg file. We will guide users to recreating the current default settings shipped with Kedro in our documentation (step 2 of the plan). These linting tools each have a CLI, though they can also be integrated with most common IDEs through easily-installed plugins. Again, this will be added to our documentation. In the case of pytest, users can switch to using the pytest CLI very easily, as The advantage of this alternative is that it captures the major advantages of deprecation outlined in the arguments above. I estimate that implementing these changes would take an experienced Kedro developer one week. In summary, users can migrate by replicating the configuration for each tool and using their interfaces independently, which we will guide them in doing in newly written documentation. Benefit:
Cost:
2. Users Install our 'Dev Plugin'If we decide to implement step 3 of the plan, users will be able to install the plugin, which bundles both In this case, the user will not need to undergo any migration beyond installing the plugin; `kedro lint' and 'kedro test' will be available as before. Benefit:
Cost:
|
This is a fair view. However, why should we even "add demos for implementing testing and linting in a Kedro project to the documentation" in that case? I'd rather a user who "knows the value of testing and linting code" choose their own tools and read the respective docs, so there's nothing for us to maintain here. Otherwise, Kedro maintainers will spend time resolving issues setting up On that note, I think the plan is missing the bit on how to make sure the project template is still linted (somehow). |
This write-up and evaluation is stellar 🎉 An additional benefit is a simpler starter for new users. If I quickly scan the documentation and starters it would mean that we would be making the following changes (if I haven't forgotten anything):
I like the documentation addition because in future, the section on testing could be expanded to cover #1271. I think @deepyaman's comment is the only thing left to cover, which is how do we make sure that we deliver linted and tested starters to our users without those configuration files being present in the starters. |
While I was walking, I realised there are more changes 😂 What should happen to |
Slightly playing devil's advocate, but something I think we're missing here is what if a user doesn't know the value of testing and linting? Having these commands shipped with kedro does increase discoverability of these notions. If I'm completely new to writing data science pipelines I might not even know about testing and linting, let alone be equipped to choose which tools I should choose or how to set them up. Are we setting users on the right path if we provide them with a project template that doesn't even have a IMO the value of these tools being built into kedro is most significant to inexperienced users, and so any documentation that effectively replaces them should also cater to that user group. Just to clarify on alternative 2 (Users Install our 'Dev Plugin'), which I personally still like: this can be done with essentially zero development effort and no ongoing maintenance if we outsource it to the community and make it a community-maintained plugin. And then our docs can just say "linting code is good. If you want a standard set of tools that help with this then we recommend this plugin". See #1622. If there's no demand for the plugin then it won't be created, which is fine. I don't think we need to do anything about it ourselves right away. That would still leave open the question of what standards we should be linting our own starter templates to and how to do it though. |
Can't we just keep the linting and testing setup in the starters as is now and use that as an example for users to show how they can configure their project to have linting and testing? I realise that with this approach we don't clean up as much, but I feel this would solve the problem of how we keep the starters in good shape and have concrete examples for users how to set up linting and testing. The only change is that we don't lint and test by doing |
In technical design (14/09/22) it was concluded that both commands should be deprecated. The following action items were agreed: 1 (#1619 and #1620). Short guides for linting and testing should be written and added to the docs. Depending on the traffic metrics for this part of the docs, a next step of creating either a lint / test starter* (see below) or a plugin will be considered. * @yetudada proposed moving the elements needed to add both commands to a starter project. This may provide significant advantages in ease of maintenance relative to the plugin option. |
Investigate what the alternatives would be for users currently using
kedro test
andkedro lint
.Write up a plan to deprecating
kedro lint
andkedro test
(including how the users can "migrate" to these alternatives) or alternatively summarise why those commands shouldn't be deprecated.Also keep in mind that the alternatives should still make it possible to have a linted/testable Kedro template.
The text was updated successfully, but these errors were encountered: