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.

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 we keep the kedro.extras namespace? #1800

Closed
noklam opened this issue Aug 22, 2022 · 3 comments
Closed

Should we keep the kedro.extras namespace? #1800

noklam opened this issue Aug 22, 2022 · 3 comments

Comments

@noklam
Copy link
Contributor

noklam commented Aug 22, 2022

More Context

A thread to keep relevant discussion tracked, this change is not urgent, but should be something considered before 0.19 is out.

Do we still want to keep this kedro.extras namespace?

Comments in #1763

I think %load_ext kedro.ipython is not as good as %load_ext kedro because it's not as short and clear. If you're doing %load_ext then you know you're trying to load an IPython extension, so the extra .ipython seems redundant to me. Looking through a list of IPython extensions this seems like a common way to name them also, e.g. the SQLAlchemy IPython extensions is loaded as %load_ext sql and not %load_ext sql.ipython. wdyt?

As for removing extras entirely, absolutely would probably be good to do that - it's always been the vague intention after datasets have moved. This would need to be only in develop though. Also, in this case we need to figure out where exactly the stuff inside extras/extensions/ (which includes .png files) should live. Maybe a new directory kedro/ipython/ or maybe inside kedro/framework/ipython/?

The reason I say it would _probably be good to remove extras is that possibly in the future we would extend #1563 to move spark hooks (and maybe rich logging hooks?) to live inside the kedro package somewhere. So we might need to figure out where those belong also, and then maybe extras makes sense again? I don't know.

Overall, I think:

  • leave all the code where it is in main and just alias it with the from .extras.extensions.ipython import load_ipython_extension import
  • open a ticket to probably move it out of extras to somewhere else (for 0.19; should take into consideration the above point about other stuff that might exist)

Originally posted by @AntonyMilneQB in #1763 (comment)

@deepyaman deepyaman changed the title Should we keep ‘kedro.extras` namespace Should we keep the kedro.extras namespace? Sep 20, 2022
@merelcht
Copy link
Member

merelcht commented Sep 5, 2023

As far as I'm aware this ticket is outdated and the answer to "Should we keep the kedro.extras namespace?" is no, because in 0.19.0 there will be nothing left there. Do you agree with that @noklam ?

@noklam
Copy link
Contributor Author

noklam commented Sep 15, 2023

  1. extras.extensions.ipython
  2. extras.logging(would be removed)
  3. extras.datasets (likely to be removed).
  4. @AntonyMilneQB mentioned we may need it to keep it for Spark's hook Initialise PySpark using hooks rather than custom context #1563

@merelcht I agree we should remove the namespace. I checked both logging/datasets/ipython have proper deprecation warnings already, so it's safe to remove it. For the comments that should we have "native" hooks in Kedro framework, we still haven't implemented it and so far we just keep a SparkHook in starters.

@merelcht
Copy link
Member

Closing this, because we will not be keeping the kedro.extras namespace.

@merelcht merelcht closed this as not planned Won't fix, can't repro, duplicate, stale Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants