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

✨ feat: Split code rust, for run as python lib and rust lib #167

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

Conversation

Miuler
Copy link

@Miuler Miuler commented Jul 18, 2023

Split the code rust, for run as python lib and rust lib, to be able to publish in both crates and pypi.

Fixes #24

@Miuler Miuler force-pushed the main branch 2 times, most recently from 9db8696 to 1250302 Compare July 18, 2023 23:04
@Miuler Miuler marked this pull request as ready for review July 18, 2023 23:08
@Miuler
Copy link
Author

Miuler commented Jul 18, 2023

Correcting sdist, they change this:

recursive-include src *.rs

for this:

recursive-include py-tiktoken *.rs
recursive-include rs-tiktoken *.rs

@Miuler
Copy link
Author

Miuler commented Jul 19, 2023

Hi @hauntsaninja, this is a first version, without changes in the core, only separating the rust in 2, a rs-tiktoken thinking in creating the crate and another py-tiktoken that is the binding for python.

Then you can think about creating more versions like one for java something like jvm-tiktoken because it is really for you to use it from java, scala, etc, but first refine the core that should be in rs-tiktoken.

@Miuler
Copy link
Author

Miuler commented Jul 19, 2023

@hauntsaninja do you think I should add more changes to this PR to have a fully functional rust version?

@Miuler
Copy link
Author

Miuler commented Jul 19, 2023

The MANIFEST.in is fixed and I also separated the workflows for aarch64 into 2:

build_wheels_aarch64_glibc:

y

build_wheels_aarch64_musl:

that's the way it is:

Captura desde 2023-07-19 13-20-39

because I see that it is giving an error commits before my changes, this happens in arch64 but the versions that use musl instead of glibc.

Should we deactivate the musl? is there really a need to support aarch64 with musl ?

@Miuler Miuler force-pushed the main branch 2 times, most recently from 2a19ff3 to 145a88e Compare July 20, 2023 07:30
@Miuler
Copy link
Author

Miuler commented Jul 21, 2023

What happen? Did I make a mistake? any comments? Is there anything I need to change in order for my PR to be considered?

This commit restructures the project from a single-crate workspace into a multi-crate workspace, dividing it into 'rs-tiktoken' and 'py-tiktoken'. This is done to improve the clarity of the organization of the codebase and make the Rust and Python modules separate for easier code maintenance. The setup.py is also updated to reflect these changes in the directory structure.

Refs: openai#24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tiktoken not published to cargo
1 participant