Alias Kedro IPython extension to kedro.ipython

[antonymilne]

Description

Closes #1763.

With many apologies to @noklam for the original work done in #1794 trying to alias kedro.extras.extensions.ipython to just kedro, it turns out to be much better to alias as kedro.ipython as we originally thought. The pattern %load_ext name.ipython seems to be about as common as %load_ext name. Its only disadvantage is a few extra characters. The main advantage is that it feels cleaner, there's no breaking import linter contracts, and there's no danger that importing kedro could possibly import IPython or have other undesired side-effects like setting up logging.

Note. We should delete kedro/extras/extensions/ from develop after this has merged.

Note to reviewers

I recommend reviewing this commit by commit since I moved a lot of code from one file to another. This will make it clear when it's just code that's moved or where I've actually changed something. The only real change to code itself is in 637053e, where the definition of the line magic and imports and moved to top level.

Development notes

Tested:

• the new alias works
• backwards compatibility of kedro.extras.extensions.ipython
• packaging kedro still includes the logo png files thanks to MANIFEST.in
• deprecation warning shows:

Note that doing the imports this way round (kedro.extras.extensions.ipython imports kedro.ipython and not vice versa) is much neater because it will allow us to remove the file from develop straight away. This way, as we modify the IPython extension going forward, the file we'll be modifying is the new correct one rather than the old deprecated one.

Checklist

• Read the contributing guidelines
• 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

[noklam]

Thanks! I think %load_ext kedro.ipython is a better solution.

After hustling with the original proposal %load_ext kedro, I think we should be very careful about any top-level import alias i.e. import kedro. I am now more cautious about the idea in #712, due to the way that Python Import system works, it's very dangerous to put things in the top-level. It now make more sense to me why Django do django.shortcuts instead of just django.

[merelcht]

Great work and thanks for explaining so clearly why just kedro as alias isn't such a good solution after all @noklam @AntonyMilneQB 👍

Should we keep one test to check %load_ext kedro.extras.extensions.ipython still works until it's deprecated?

[antonymilne]

@MerelTheisenQB good thinking, done in 645ce1f!

Actually I've just found out that most of the tests in this file haven't really doing anything useful for a long time. The mocking is all wrong and so not testing much... I'm going to make a new ticket to fix this up properly.

[noklam]

I found test_logging.py and test_pipeline_registry.py interfere with test_ipython.py, I am slightly worried that there are side-effects that we don't understand well with del sys.modules[xxx].

pytest tests/ipython/test_ipython.py tests/framework/project/test_logging.py will pass
pytest tests/framework/project/test_logging.py tests/ipython/test_ipython.py will fail
pytest tests/framework/project/test_logging.py tests/ipython/test_ipython.py --numprocesses 4 --dist loadfile will also pass

[noklam]

Suggested change

	mocker.patch("kedro.framework.startup.configure_project")
	mocker.patch("kedro.ipython.configure_project")

This is the wrong patch that causing most of the confusion. @AntonyMilneQB

[antonymilne]

Ahh, if I change this then can I remove the no pragma and the tests pass?

[noklam]

Let's address this altogether at #1839