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

[Tokenizers] Added support for conda-forge, archive installations #782

Merged
merged 2 commits into from
Dec 12, 2023

Conversation

ilya-lavrenov
Copy link
Contributor

No description provided.

@ilya-lavrenov ilya-lavrenov requested a review from a team as a code owner December 12, 2023 08:52
@github-actions github-actions bot added category: build OpenVINO cmake script / infra category: custom operations OpenVINO Runtime Extension with custom operations labels Dec 12, 2023
sys.exit(f"Error: extension does not support the platform {sys.platform}")

# conda-forge case
conda_forge_lib_path = Path(os.path.join(os.path.dirname(__file__), "..", "..", ".."))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be better to search extension in the os.environ["PATH"]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if conda env is not activated, it will not work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a scenario when a user installs openvino_tokenizers with conda, but uses it without activating any conda env?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

potentially I can use /abs/path/to/conda/env/python and openvino_tokenizers will find libraries.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be more robust. We had problems with pip install -e . when trying to find libraries relative to the __file__ path. There may be similar problems with a conda installation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

conda does not work in editable mode

renamed component core => tokenizers
@apaniukov apaniukov changed the title Added support for conda-forge, archive installations [Tokenizers] Added support for conda-forge, archive installations Dec 12, 2023
Copy link
Contributor

@apaniukov apaniukov left a comment

Choose a reason for hiding this comment

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

Merge after moving from os.path.* to Path

@ilya-lavrenov
Copy link
Contributor Author

Merge after moving from os.path.* to Path

I'll merge as is and fix comments in next PR #784

@ilya-lavrenov ilya-lavrenov merged commit d679ea4 into openvinotoolkit:master Dec 12, 2023
6 checks passed
@ilya-lavrenov ilya-lavrenov deleted the conda-forge branch December 12, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: build OpenVINO cmake script / infra category: custom operations OpenVINO Runtime Extension with custom operations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants