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

Add python support to ITT API #145

Merged
merged 13 commits into from
Jun 20, 2024
Merged

Add python support to ITT API #145

merged 13 commits into from
Jun 20, 2024

Conversation

eparshut
Copy link
Contributor

Add Python binding to ITT API.
Many kudos to @esuldin for the work done and his contribution.

@eparshut eparshut mentioned this pull request May 14, 2024
Copy link
Contributor

@abrown abrown left a comment

Choose a reason for hiding this comment

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

@eparshut asked me to take a look at this so here are some thoughts:

  • First, I think this is great! Having control over collection in more languages is extremely helpful; I wasn't even aware this existed until I saw it upstreamed here.
  • A naming nit: I would probably go with python as the top-level directory for this code to match the rust directory. And I would have expected the PyPI package to be named ittapi, but it's probably too late to change that.
  • I noticed different coding styles throughout; maybe we should add a formatter pass in CI
  • Which brings me to CI: this addition would really benefit from building itself in a GitHub workflow, running the tests, running lints, etc.

pyitt/LICENSE Outdated Show resolved Hide resolved
pyitt/pyproject.toml Outdated Show resolved Hide resolved
@lubomirov
Copy link

lubomirov commented May 23, 2024

Let me ditto @abrown here:

  1. Let's use python for the subfolder and not pyitt.
  2. Transfer CI/CD elements from @esuldin's implementation.
  3. Name of the package in the "pip" repositories should be ittapi
  4. Let's point Egor's contribution here through his Intel's email - egor.suldin@intel.com

python/pyproject.toml Outdated Show resolved Hide resolved
python/README.md Outdated Show resolved Hide resolved
python/README.md Outdated Show resolved Hide resolved
python/pyproject.toml Show resolved Hide resolved
@esuldin
Copy link
Contributor

esuldin commented May 30, 2024

4. Let's point Egor's contribution here through his Intel's email - `egor.suldin@intel.com`

The usage of Intel email address is not align with Own-Time Open Source Project Approval that pyitt has. Therefore, please use original email address or just remove it at all from pyproject.toml.

python/setup.py Outdated Show resolved Hide resolved
@eparshut eparshut changed the title [WIP] Add python support to ITT API Add python support to ITT API Jun 19, 2024
@eparshut eparshut merged commit 3346299 into master Jun 20, 2024
12 checks passed
@eparshut eparshut deleted the dev_python_ittapi branch June 20, 2024 12:12
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.

7 participants