search

kedro-org / kedro

Kedro is a toolbox for production-ready data science. It uses software engineering best practices to help you create data engineering and data science pipelines that are reproducible, maintainable, and modular.
https://kedro.org
Apache License 2.0
9.49k stars 874 forks source link

Kedro new starter CLI : user_input.lower() #3783

Open Ipsedo opened 3 months ago

Ipsedo commented 3 months ago

Description

I use kedro starter CLI configured by prompt.yml with regex check and I think I found one issue : the user inputs are systematically transformed to lowercase : https://github.com/kedro-org/kedro/blob/main/kedro/framework/cli/starters.py#L957 The result is that I can't restrict user input to uppercase or lowercase

Context

I want to restrict user input to uppercase

Steps to Reproduce

  1. create a cookiecutter template for kedro (regarding your need)
  2. create your cookiecutter.json with project_name as unique prompt
  3. create your prompt.yml with an entry for project_name
  4. set the regex_validator field to "[A-Z_]+"
  5. start kedro new --starter=./my_starter and answer MY_PROJECT for project_name prompt
  6. It will fail due to the user_input.lower() in kedro/framework/cli/starters.py line 957

Expected Result

The regex and the input must match

Actual Result

The user input is refused

Your Environment

Python : 3.9.13 Kedro : 0.19.3 OS : Linux (distribution and kernel version confidential)

noklam commented 3 months ago

I think that is by design that we don't allow upper case. As I understand this is the convention for Python community.

https://gist.github.com/etigui/7600441926e73c3385057718c2fdef8e

However the error may not be clear enough, curious if you are creating your own starters?

Ipsedo commented 3 months ago

I know about python naming convention, the point is not here : the regex of prompt for project name in uppercase was just for the example. In reality I use the regex for a variable value in the project which needs to be in uppercase, your code is clear : you apply lower() method on the user input which is not correct.

noklam commented 3 months ago

@lpsedo got it, would you like to send a PR to fix this? It should be a relative simple one happy to merge this.

Ipsedo commented 3 months ago

Sure, I will potentially make the PR this weekend

merelcht commented 1 month ago

Hi @Ipsedo are you still interested in creating a PR for this?

parthmshah1302 commented 2 hours ago

@merelcht I would like to work on this - do I need to be assigned first or should I submit a PR directly?