Private methods _is_project and _find_kedro_project are made public

[ravi-kumar-pilla]

Description

Resolves #4523

Development notes

• Made all instances of the private methods _is_project and _find_kedro_project public.
• Renamed is_project to is_kedro_project
• Updated tests

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

• Read the contributing guidelines
• Signed off each commit with a Developer Certificate of Origin (DCO)
• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added a description of this change in the RELEASE.md file
• Added tests to cover my changes
• Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

[ravi-kumar-pilla]

Hi Team, I did not add any deprecations in this PR which will be merged to develop. I am not exactly sure how develop merges to main or vice-versa. From the docs, I found develop is for breaking changes, so this PR will break things for plugin developers. Should I raise another PR with a deprecation warning in these private methods as below and merge it to main before 1.0 ?

 warnings.warn(
        "`_find_kedro_project()` has been deprecated and will be removed in Kedro 1.0.0. Please switch to using public method `find_kedro_project()`",
        KedroDeprecationWarning,
    )

cc: @ankatiyar
Thank you

[merelcht]

@ravi-kumar-pilla We haven't done a change like this often (or ever?), so we'll need to decide how we want to approach it here.

In my opinion, since these were private methods and not supposed to be used by users, I would argue it's not necessary to go through a full deprecation process. I think explicitly adding the change in migration notes for 1.0.0 is enough. The change from private to public isn't even strictly considered a breaking change, so we're already being more cautious by doing this in a breaking release.

[ankatiyar]

Overall code looks good!

• I'm still not a 100% about making _split_params public just because it's used by kedro-airflow since we can just make the plugin parse the parameters with its own implementation. (Also side note, would anyone want to pass extra params to the session through airflow CLI kedro airflow create --params -> DAG that passes it to the session instead of putting them in the configuration during deployment?)
• Since it's private methods used by our own code, multiple times, the deprecation warnings will flood the terminal, I think we should add it in the release notes/migration guide and/or post in #plugins-integrations for the plugin developers

[rashidakanchwala]

I also agree with Ankita on the _split_params func. @merelcht let us know how you feel. Also agree with both @ankatiyar and @merelcht and not having the Deprecation Warnings and just adding it to release notes.

[ravi-kumar-pilla]

Thank you @ankatiyar , @merelcht and @rashidakanchwala for the comments.

So based on the suggestions, we will only inform the users of this change via the migration and release notes.

For the _split_params I do feel it is very much related to kedro as the logic is specific to how params are passed in kedro. I do see an issue in making this public if we decide on changing how params are passed making this a breaking API change. Also, I think it is not about one/many plugin(s) using the method (which might change in future), but if the team feels it as a public utility function we should expose it.

Further on this, I think is_project seems internal function to me which is used by find_kedro_project. So, I am not 100% clear on what should we make public and what to keep private. But I am convinced on making find_kedro_project public. Others (is_project and split_params), I am happy to go either way based on what the team feels.

Thank you

[merelcht]

The reason I said we can make _split_params public was to make it consistent with the other utility functions split_string and split_node_name. However, the point raised about this being tied to how parameters work in Kedro and then making it harder to revert this in future if needed is a really good one. Since it's just kedro-airflow using it, I agree with @ankatiyar we could always implement a similar function on that side.

[merelcht]

For _is_project I thought this was also used by other plugins like kedro-mlflow right? I'd still make that public. One reflection though is that maybe we should rename it to is_kedro_project, because that would make it easier to understand what this does in other plugins.