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.88k stars 895 forks source link

Deprecate (mark for future removal) `get_pkg_version` from the public API #3789

Closed noklam closed 3 months ago

noklam commented 5 months ago

Context

          @merelcht It didn't change by #3742.

https://github.com/kedro-org/kedro/blob/a91ccdbecb5541490b5146b6179ab4c621b9ea73/kedro/framework/cli/utils.py#L204-L218

I just check again, this function is not used anywhere in framework, but only tests. (last change is 4 years ago). Maybe at some points it was used for kedro build-reqs? I suggest we deprecate this function since it's a public function and move it to test.

Originally posted by @noklam in https://github.com/kedro-org/kedro/issues/2912#issuecomment-2031643218

Remove the function from public API and move it to test only.

noklam commented 5 months ago

One question here, we tend to do the breaking change later in develop to avoid the code diverge too much. At the same time should we add the deprecated warnings earlier in 0.19 to inform user?

This is not important for this particular issue, because I think no one use it (hopefully), but it is a valid question for what should we do in general?

astrojuanlu commented 5 months ago

should we add the deprecated warnings earlier in 0.19 to inform user?

Yes

noklam commented 5 months ago

Do we need a subticket for deprecation? Or this is ready to go in backlog grooming.

On Thu, 11 Apr 2024, 15:32 Juan Luis Cano Rodríguez, < @.***> wrote:

should we add the deprecated warnings earlier in 0.19 to inform user?

Yes

— Reply to this email directly, view it on GitHub https://github.com/kedro-org/kedro/issues/3789#issuecomment-2049834401, or unsubscribe https://github.com/notifications/unsubscribe-auth/AELAWL6RQQZNPW2IXZXHCFDY42NJXAVCNFSM6AAAAABF4VKBCOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBZHAZTINBQGE . You are receiving this because you authored the thread.Message ID: @.***>

astrojuanlu commented 5 months ago

I'd say let's rename this ticket to "Deprecate get_pkg_version" and groom it

noklam commented 5 months ago

Actually the tests are obsoleted and we should remove it altogether class TestCliUtils in test_cli.py.

astrojuanlu commented 4 months ago

Funny bit of insight found in the source code of kedro-mlflow:

            # if it is a local relative path, make it absolute
            # .resolve() does not work well on windows
            # .absolute is undocumented and have known bugs
            # Path.cwd() / uri is the recommend way by core developpers.
            # See : https://discuss.python.org/t/pathlib-absolute-vs-resolve/2573/6

Not sure how up to date it is though (the thread is from 2019).