vdk-ipython: package ipython correctly #2471
Conversation
|
The PR include changes not just in vdk-ipython. Maybe you need to rebase ? Is there related Github issue to the PR ? Can you describe in a bit more details the bug? |
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
Signed-off-by: murphp15 <[email protected]>
murphp15
force-pushed
the
person/murphp15/package_ipython_correctly
branch
from
504a128 to
e834930
Compare
|
@tozka rebased. sorry about that. |
| @@ -16,7 +16,6 @@ | |||
| long_description_content_type="text/markdown", | |||
| install_requires=["vdk-core", "iPython"], | |||
| package_dir={"": "src"}, | |||
| packages=setuptools.find_namespace_packages(where="src"), | |||
There was a problem hiding this comment.
I don't understand the fix. Removing it seems like bad idea. We keep sources in "src" folder so we use setuptools.find_namespace_packages to automatically find all the Python packages within the directory
There was a problem hiding this comment.
@tozka there are no packages in that python project. There is just a single python file at the root of source.
There was a problem hiding this comment.
Oh, that's not following the plugin templates.
The source code should be in in "src" directory in package starting with vdk.plugin.<plugin-name>....
That's the structure of all other plugins . It's the one setup if a plugin is created using the cookiecutter - https://github.com/tozka/cookiecutter-vdk-plugin/tree/main
But I don't get the problems? Which files are not included? And why do we need them?
There was a problem hiding this comment.
vdk_ipython.py is at the root of the source folder. It is not in a package so if that line is in the setup.py then this file is dropped.
@duyguHsnHsn do you think we could put the plugin in a package or would it make it less usable for customers do you think?
There was a problem hiding this comment.
I think I got the problem.
find_namespace_packages doesn't include files that are not in a directory (aka package), and those that are at the root level specified are ignored. I find this a bit strange to be honest. And slightly problematic. I would expect find_namespace_packages to assume root level files are in the global namespace/package.
The current approach you are doing would also have a namespace/package. If you unzip the tar you will see the file is inside a "src" folder . So a user would need to do
from src import vdk_ipython
you can test it yourself by installing pip install dist/vdk-ipython-0.1.0.tar.gz
The test pass because they assume "src" structure is kept and set src on the PythonPath and ignore setup.py
There was a problem hiding this comment.
I am in favour of changing it to src/vdk/plugin/ipython.py
Then when the extension is loaded it would look like load_ext vdk.plugin.ipython
|
@tozka there are no packages in that python project. There is just a single python file at the root of source. |
I favour automated tests? Why isn't our Gitlab CI not catching the issue. But for any manual tests, please describe them in such a way that I (or anyone) can reproduce them. It's useful since |
|
@duyguHsnHsn do you have tests on the juypter UI, why did they not catch this issue? |
Why
There is a bug in the current release of vdk-ipython. It is not including the files at the root of the source folder.
What
Remove the line from the setup.py so files at the root are included.
How was this tested?
Packaged locally and all looks good
Steps:
python setup.py sdist --formats=gztar