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.

Sign up for GitHub

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

vdk-ipython: package ipython correctly #2471

Closed
wants to merge 15 commits into from
Closed

vdk-ipython: package ipython correctly #2471

Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8f0ff8b
control-service: use full url for control service
Jun 21, 2023
fb5ab36
control-service: use full url for control service
Jun 21, 2023
c166963
control-service: use full url for control service
Jun 21, 2023
ca7fbef
control-service: use full url for control service
Jun 21, 2023
ff90761
control-service:fix failing pipelines
Jun 27, 2023
f5ae9e3
control-service:fix failing pipelines
Jun 27, 2023
e960889
control-service:fix failing pipelines
Jun 27, 2023
5ce8400
control-service:fix failing pipelines
Jun 27, 2023
1139082
control-service: test trino full url
Jul 24, 2023
bb07a33
control-service: test trino full url
Jul 24, 2023
05be46c
control-service: test trino full url
Jul 24, 2023
6fef7f8
control-service: test trino full url
Jul 24, 2023
80a24d4
control-service: test trino full url
Jul 24, 2023
e834930
vdk-ipython: package ipython correctly
Jul 25, 2023
38bb0c5
csv
Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion projects/vdk-plugins/vdk-ipython/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tozka there are no packages in that python project. There is just a single python file at the root of source.

Copy link
Collaborator

@antoniivanov antoniivanov Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

@antoniivanov antoniivanov Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

entry_points={"vdk.plugin.run": ["ipython = vdk_ipython"]},
classifiers=[
"Development Status :: 2 - Pre-Alpha",
Expand Down