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

Use maturin as build-backend and update Release PyPi GitHub workflow #165

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yvan-sraka
Copy link
Contributor

@yvan-sraka yvan-sraka commented Jan 31, 2025

Closes #105
Closes #75

We would want to close #176 too

@yvan-sraka yvan-sraka requested review from torymur and rlouf January 31, 2025 15:13
@yvan-sraka yvan-sraka self-assigned this Jan 31, 2025
@yvan-sraka yvan-sraka linked an issue Feb 3, 2025 that may be closed by this pull request
pyproject.toml Outdated Show resolved Hide resolved
@yvan-sraka yvan-sraka force-pushed the maturin branch 3 times, most recently from c1f307d to 7d7d102 Compare February 4, 2025 12:43
@torymur
Copy link
Contributor

torymur commented Feb 4, 2025

Agree with the steps, especially this:

merging jobs wasn't the best approach

And we need to take into consideration, that currently for macos builds there is inconsistency #168, I will fix this using macos-latest: #169
But this reopens the issue to have builds both for macos-14 & macos-15: #75

Maybe having jobs per each os with strategy matrix per python version and target:

  release:
    name: Release to PyPI
    runs-on: ubuntu-latest
    needs:
      - linux
      - windows
      - macos-14
      - macos-15
      - linux-cross

@yvan-sraka yvan-sraka requested a review from torymur February 4, 2025 12:45
@yvan-sraka yvan-sraka marked this pull request as ready for review February 4, 2025 12:46
@yvan-sraka
Copy link
Contributor Author

yvan-sraka commented Feb 4, 2025

 __________________ ERROR collecting tests/test_json_schema.py __________________
ImportError while importing test module '/home/runner/work/outlines-core/outlines-core/tests/test_json_schema.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/_pytest/python.py:493: in importtestmodule
    mod = import_path(
/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/_pytest/pathlib.py:549: in import_path
    mod = _import_module_using_spec(
/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/_pytest/pathlib.py:715: in _import_module_using_spec
    spec.loader.exec_module(mod)  # type: ignore[union-attr]
/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/_pytest/assertion/rewrite.py:184: in exec_module
    exec(co, module.__dict__)
tests/test_json_schema.py:5: in <module>
    from outlines_core.json_schema import build_regex_from_schema
E   ModuleNotFoundError: No module named 'outlines_core.json_schema'       

src/python_bindings/mod.rs Outdated Show resolved Hide resolved
@yvan-sraka yvan-sraka force-pushed the maturin branch 2 times, most recently from e180ccc to df00717 Compare February 4, 2025 23:21
@yvan-sraka yvan-sraka requested a review from torymur February 4, 2025 23:22
@yvan-sraka
Copy link
Contributor Author

I gave a try to % maturin generate-ci > .github/workflows/release_pypi.yaml, WDYT?

@yvan-sraka
Copy link
Contributor Author

Run python -m coverage combine
Combined data file .coverage.8cc0c411e24d7bac9c3565f8296dcf91
No source for code: '/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/outlines_core/__init__.py'.
Error: Process completed with exit code 1.

README.md Outdated Show resolved Hide resolved
@torymur
Copy link
Contributor

torymur commented Feb 5, 2025

To address questions:

  1. generated release file could be a good starting point (maybe, if it's easier), but it can't be used as it is, we need to do match exactly what we currently have and then iterate if needed
  2. scopes seem okay, tests are failing, I think you might need to adjust the testing workflow to do maturin develop before running them

@yvan-sraka yvan-sraka force-pushed the maturin branch 4 times, most recently from aef5905 to 4782a04 Compare February 5, 2025 14:21
@torymur torymur force-pushed the maturin branch 14 times, most recently from 1869584 to 60d005a Compare February 11, 2025 18:15
@torymur
Copy link
Contributor

torymur commented Feb 11, 2025

Fortunately, PR was quickly merged in tokenizers, so we're opting out from openssl in favor of rustls, which drastically simplifies absolutely everything for us 🎉

Some notes:

All is green now: https://github.com/dottxt-ai/outlines-core/actions/runs/13269673921/job/37045906346?pr=165

A couple more things to check, but we're close here.

@torymur torymur self-assigned this Feb 11, 2025
@torymur torymur added the CI label Feb 11, 2025
@torymur torymur force-pushed the maturin branch 10 times, most recently from aa1ef53 to e0f9be3 Compare February 12, 2025 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants