fix: python dependency specification

[tychoish]

No description provided.

[tychoish]

I expect the environment/optional dependency thing something we should discuss (do we want to force pulling in pandas and polars? Probably dev dependencies are going to lead to some kind of issue...

[talagluck]

I don't think we want to force pulling in polars and I'm not sure about pandas. The more dev dependencies we have the harder this will be to navigate (ran into this a lot at a previous place) and pip's dependency resolver can be kind of heavy, so unless something is strictly necessary for core functionality, it should be optional

[universalmind303]

in addition to making things optional dependencies, we should also properly handle when the deps are not installed.

It should error instead of panicking

this is what polars does if you try df.to_arrow() without pyarrow installed

Traceback (most recent call last):
  File "/Users/me/Development/glaredb/test_pl.py", line 3, in <module>
    pl.DataFrame({"foo": [1, 2, 3]}).to_arrow()
  File "/Users/me/Development/glaredb/.venv/lib/python3.11/site-packages/polars/dataframe/frame.py", line 1970, in to_arrow
    record_batches = self._df.to_arrow()
                     ^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named 'pyarrow'

we instead just panic.

[tychoish]

in addition to making things optional dependencies, we should also properly handle when the deps are not installed.

It should error instead of panicking

Totally agree. I think we should probably do that work as part of a seperate ticket just to fix the bug, incase we do a point release early next week.

Do you, @universalmind303 or @talagluck, want to take this over? g

[tychoish]

I spent a couple hours playing with this, and I have something that maybe works:

• installs build time dependencies and test/runtime dependencies into different virtualvens that are scoped to the bindings.
• installs the extra dependencies (I think we want to have this off by default?
• the install succeeds but it has a patchelf error message. I was able to run something locally, but I didn't kick the tires very hard.

I'm not sure how to do the better error messages you're interested in Cory, but that'd definitely be a good thing to add.

[tychoish]

I did a different update to the pytests, and this should just hit the python deps. There was a merge issue for a while, but I've cherry-picked and force pushed this for the moment.

Remaining work should be buildsystem related, I believe.

[tychoish]

@universalmind303 could you take another look.

[greyscaled]

I just ran this from the repository root dir - should we add a dependency on venv for pytest in the root Justfile?

greyb ~/Repos/glaredb [tycho/python-deps] $ just pytest
.venv/bin/poetry -C tests lock --no-update
sh: .venv/bin/poetry: No such file or directory
error: Recipe `pytest` failed on line 131 with exit code 127

[greyscaled]

After the previous comment, I ran just venv and got this:

greyb ~/Repos/glaredb [tycho/python-deps] $ just venv
python3 -c "import virtualenv" || python3 -m pip --quiet install virtualenv
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'virtualenv'
error: externally-managed-environment

× This environment is externally managed
╰─> To install Python packages system-wide, try brew install
    xyz, where xyz is the package you are trying to
    install.
    
    If you wish to install a non-brew-packaged Python package,
    create a virtual environment using python3 -m venv path/to/venv.
    Then use path/to/venv/bin/python and path/to/venv/bin/pip.
    
    If you wish to install a non-brew packaged Python application,
    it may be easiest to use pipx install xyz, which will manage a
    virtual environment for you. Make sure you have pipx installed.

note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
hint: See PEP 668 for the detailed specification.
error: Recipe `venv` failed on line 124 with exit code 1

I recognize that it's prompting me to install virtualenv but I'm wondering if this is expected, and if so, can we document this somewhere (if it's not already)

I must admit the command itself: python3 -c "import virtualenv" || python3 -m pip --quiet install virtualenv makes this feel unexpected.

[tychoish]

@talagluck @universalmind303 can you take another look at this?

[talagluck]

Overall the approach feels reasonable, and I was able to successfully run the code, but I'll defer to Cory for the final sign-off.