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

chore(python): Create .venv in repo root #10789

Merged
merged 3 commits into from
Aug 30, 2023
Merged

chore(python): Create .venv in repo root #10789

merged 3 commits into from
Aug 30, 2023

Conversation

stinodego
Copy link
Contributor

@stinodego stinodego commented Aug 30, 2023

Preparation for #10560

Building the user guide requires mkdocs, which is a Python dependency. After moving the docs into this repo, we'll also have Python files (code snippets) in the docs folder. Because of this, we'll want to run some Python commands from the root of the repo, so it makes sense to move the Python virtual environment to the repo root.

This shouldn't impact anyone's workflow. All existing make commands should still work identically.

Changes

  • Add top-level Makefile including the following commands (can add more commands later):
    • .venv
    • requirements
    • clean
    • help
  • Adjust the Python Makefile to dispatch to the top-level Makefile for .venv / requirements commands and use the right venv path.
  • Update other .venv references
  • Update bytecode parser workflow to not run on the main branch - there is no need since nothing is being cached.
  • Update dprint plugin versions

@github-actions github-actions bot added internal An internal refactor or improvement python Related to Python Polars labels Aug 30, 2023
@ritchie46
Copy link
Member

ritchie46 commented Aug 30, 2023

I often call .venv/bin/pytest directly from the py-polars directory. Can we symlink it maybe?

Do we really need the user-guide in here though? That's a lot of code and maybe images? I really don't want to bloat this one as it will grow big already.

@stinodego
Copy link
Contributor Author

stinodego commented Aug 30, 2023

I often call .venv/bin/pytest directly from the py-polars directory. Can we symlink it maybe?

In that case, I'd recommend either:

  1. Activate your virtual environment. This will give direct access to all the binaries in the virtual environment, i.e. you can run pytest using pytest. Most IDEs will allow you to activate your virtual environment automatically on startup (that's what I do). Or you could type source .venv/bin/activate.
  2. Add a bash alias: alias pytest='../.venv/bin/pytest. Saves you from having to type .venv/bin all the time. Or shorten pytest to pt and save even more keystrokes.

Either of these options seem like much better idea than adding a symlink to the top-level venv.

Do we really need the user-guide in here though? That's a lot of code and maybe images? I really don't want to bloat this one as it will grow big already.

There is really no good way to keep the user guide up-to-date with the code otherwise. Right now, the guide is instantly outdated anytime we publish a new release. Also, the guide cannot be versioned correctly with the main branch.

The more extensive our user guide, the more quickly it will be outdated. So we really have to solve that problem as soon as possible by moving it into the main repo.

The amount of code and images is not too bad, actually. Right now, it's a bunch of markdown pages and 3 matplotlib images. And 45 Python code snippets.

Also, it will remove clutter from the repo as we finally have a good place to put extensive contributing information and other info.

@ritchie46
Copy link
Member

Ok.. let's then make a policy to put images in polars-static. I really don't want binary blobs in here as every update is a complete blob.

@stinodego
Copy link
Contributor Author

stinodego commented Aug 30, 2023

Ok.. let's then make a policy to put images in polars-static. I really don't want binary blobs in here as every update is a complete blob.

We can do that, though we should probably look into using Git LFS or something, as we already have a bunch of binary files in the repo right now. Here's a grep:

examples/datasets/tpc_heads/customer.feather
examples/datasets/tpc_heads/lineitem.feather
examples/datasets/tpc_heads/nation.feather
examples/datasets/tpc_heads/orders.feather
examples/datasets/tpc_heads/part.feather
examples/datasets/tpc_heads/partsupp.feather
examples/datasets/tpc_heads/region.feather
examples/datasets/tpc_heads/supplier.feather
py-polars/tests/unit/io/files/delta-table/.part-00000-e42312d7-60e5-454d-acbc-db192d220e73-c000.snappy.parquet.crc
py-polars/tests/unit/io/files/delta-table/.part-00000-e4a999da-df45-4fb0-bdc4-d999fc0f58aa-c000.snappy.parquet.crc
py-polars/tests/unit/io/files/delta-table/_delta_log/.00000000000000000000.json.crc
py-polars/tests/unit/io/files/delta-table/_delta_log/.00000000000000000001.json.crc
py-polars/tests/unit/io/files/delta-table/part-00000-e42312d7-60e5-454d-acbc-db192d220e73-c000.snappy.parquet
py-polars/tests/unit/io/files/delta-table/part-00000-e4a999da-df45-4fb0-bdc4-d999fc0f58aa-c000.snappy.parquet
py-polars/tests/unit/io/files/empty.xlsx
py-polars/tests/unit/io/files/example.xlsx
py-polars/tests/unit/io/files/foods1.ipc
py-polars/tests/unit/io/files/foods1.parquet
py-polars/tests/unit/io/files/foods2.ipc
py-polars/tests/unit/io/files/foods2.parquet
py-polars/tests/unit/io/files/gzipped.csv.gz
py-polars/tests/unit/io/files/small.parquet
py-polars/tests/unit/io/files/tz_aware.parquet

I'll put it on my backlog 😄

@ritchie46
Copy link
Member

Most if those files are generated IIRC.

@stinodego
Copy link
Contributor Author

Most if those files are generated IIRC.

They are not 😞 but let's pick that up separately.

Can I go ahead and merge this one? Then I can try to finish the user guide integration.

@ritchie46 ritchie46 merged commit 6759b53 into main Aug 30, 2023
@ritchie46 ritchie46 deleted the venv-repo-root branch August 30, 2023 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants