Introduce pyproject toml file with optional install method and dropping python 3.7 support by onuralpszr · Pull Request #180 · roboflow/supervision

onuralpszr · 2023-07-04T21:18:00Z

Description

Related issue(s):

This change introduce pyproject.toml file with poetry and allows to user only install opencv-python-headless or full version of opencv in choice.

Example

pip install supervision # this will install only headless version of opencv or
pip install supervision[full] # this will install full opencv version

Second change is dropping python 3.7 support. It is because python 3.7 is reached to EOL and well known project like pytorch and tensorflow already dropped python 3.7 in their latest versions.

Link : https://devguide.python.org/versions/
Link : https://github.com/tensorflow/tensorflow
Link: https://github.com/pytorch/pytorch

List any dependencies that are required for this change.

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

For testing pyproject.toml users needs to install poetry in their system and create & activate environment via

poetry install # this install packages
poetry shell # this activate environment

Once this PR merged, document needs to indicate we no longer give full opencv but instead you will only get headless version of opencv

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

…versions Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

…akefile Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

…nd pypi-test Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

onuralpszr · 2023-07-04T23:08:07Z

I noticed that in the discussion list (#181), we may need to introduce new mandatory or optional dependencies based on requirements and how the code is written. If we need more than one group, please let me know. Regarding optional installation, I am open to new names. I only used 'full' as a placeholder to demonstrate the installation process.

Here is the issues I notice require new dependencies

#177 - seaborn
#173 - open-mmlab/mmdetection(this one doesn't have pip install method) / open-mmlab/mmyolo (or we can add openmim and users can install based on their needs via "mim" tool)
#159 - pillow

SkalskiP · 2023-07-05T08:51:24Z

Hi, @onuralpszr 👋🏻! Massive change. Thanks a lot for helping us out.

I left a few comments, but I also have a few that are not directly died to your change.

Would it be possible to make the desktop a default installation but allow headless optionally?
Regarding [1] and your comment, if we are going to keep headless as the default then I'd rename full to desktop.
Please update the install section of the README.md 🙏🏻 There is a part where we say, Pip install the supervision package in a [3.11>=Python>=3.7](https://www.python.org/) environment.
As I said, I'm super new to .toml and poetry. After the package is built and submitted to PyPI, users can install it with pip. No poetry required?
As for additional dependencies you noticed:

#177 - It is possible to do it only with matplotlib and I'll advocate for dropping seaborn before merging to main.
#173 - Notice we do not use mmdetection directly, we only interact with results. We already have several other frameworks connectors. We assume you mmdetection result.
#159 - True. After merging this into the main we will need to add pillow.

SkalskiP · 2023-07-05T08:37:04Z

+opencv-python-headless = "^4.8.0.74"
+matplotlib = "^3.7.1"
+pyyaml = "^6.0"
+opencv-python= { version = "^4.8.0.74", optional = true }


Is it possible to make opencv-python-headless optional and opencv-python default? And if so, how would that work?

I'm asking because most of our users are probably desktop users, but there is a growing need for headless installation. I want to support both, but I still prioritize desktop.

I sent the proper commit and my comments about it.

In that case, let's bring back opencv-python as not optional. We need to install some version of opencv by default.

SkalskiP · 2023-07-05T08:42:56Z

+[tool.poetry.dependencies]
+python = ">=3.8,<3.12.0"
+numpy = "^1.20.0"
+opencv-python-headless = "^4.8.0.74"


Do we need to pin a minimal version of all packages? Up until now, we only had numpy pinned, and I don't want to force artificial constraints unless I really need to force something.

Poetry does that automatically plus I did installation in python3.8 so any version above we assume it is gonna be fine.

Poetry does that automatically

So it is not possible to just not have it?

docs: 📝 change min python 3.7 to 3.8 in readme Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

onuralpszr · 2023-07-05T12:43:25Z

Hi, @onuralpszr 👋🏻! Massive change. Thanks a lot for helping us out.

I left a few comments, but I also have a few that are not directly died to your change.

Would it be possible to make the desktop a default installation but allow headless optionally?

Regarding [1] and your comment, if we are going to keep headless as the default then I'd rename full to desktop.

Please update the install section of the README.md 🙏🏻 There is a part where we say, Pip install the supervision package in a [3.11>=Python>=3.7](https://www.python.org/) environment.

As I said, I'm super new to .toml and poetry. After the package is built and submitted to PyPI, users can install it with pip. No poetry required?

As for additional dependencies you noticed:

#177 - It is possible to do it only with matplotlib and I'll advocate for dropping seaborn before merging to main.

#173 - Notice we do not use mmdetection directly, we only interact with results. We already have several other frameworks connectors. We assume you mmdetection result.

#159 - True. After merging this into the main we will need to add pillow.

Yes I think I can do that, I am thinking "how should I do" I keep you posted in here.
okay that would be easy change to do.
That is done
people who want to only use not need poetry at all just "pip" is enough in their virtualenv. If people manage their virtualenv via poetry, they can use that too. But "poetry" is NOT must so it is fine.
For now I am only going to add pillow I guess.

onuralpszr · 2023-07-06T06:34:45Z

@SkalskiP

While I was trying to find alternative way of with keeping "opencv-python" as default. I am having a problem and that is basically I cannot make opencv-python default because I cannot delete it in case of headless install, even we install for the first time it still going to install opencv-python no matter what and that defeats the purpose of split into extras system. What I can offer is

pip install supervision  # no opencv 
pip install supervision[desktop] # opencv with GUI
pip install supervision[headless] # opencv without GUI

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

onuralpszr · 2023-07-06T06:36:40Z

For quick local testing

pip install .  # no opencv 
pip install .[desktop] # opencv with GUI
pip install .[headless] # opencv without GUI

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

SkalskiP · 2023-07-07T17:27:10Z

Hi @onuralpszr 👋🏻

Thanks a lot for your patience and willingness to look for solutions to my doubts.

We need to have some version of opencv installed. So in that case let's install opencv-python-headless by default.

SkalskiP · 2023-07-07T17:36:00Z

+opencv-python-headless = "^4.8.0.74"
+matplotlib = "^3.7.1"
+pyyaml = "^6.0"
+opencv-python= { version = "^4.8.0.74", optional = true }


In that case, let's bring back opencv-python as not optional. We need to install some version of opencv by default.

SkalskiP · 2023-07-07T17:36:40Z

+[tool.poetry.dependencies]
+python = ">=3.8,<3.12.0"
+numpy = "^1.20.0"
+opencv-python-headless = "^4.8.0.74"


Poetry does that automatically

So it is not possible to just not have it?

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

onuralpszr · 2023-07-07T17:58:28Z

@SkalskiP

1 - For versions yes just change toml file and you all good
2 - For poetry It should not affect you or others It just state that that version and above is fine
3 - I set headless as default as well.

SkalskiP · 2023-07-13T17:36:38Z

@onuralpszr looks good! I'll do a few tests, and we will merge.

onuralpszr · 2023-07-13T17:46:59Z

Awesome let me know if you need anything

SkalskiP · 2023-07-17T18:05:51Z

Hi @onuralpszr 👋🏻! I tested PR today. And it looks good. There is, however, something that I'm a bit worried about.

Our Pillow version is too high and forces the environment to restart. I wouldn't like that to happen. Could we make the required version lower? Preferably 8.4.0?

Collecting git+https://github.com/onuralpszr/supervision.git@python-toml-file-convert
  Cloning https://github.com/onuralpszr/supervision.git (to revision python-toml-file-convert) to /tmp/pip-req-build-m2emcu19
  Running command git clone --filter=blob:none --quiet https://github.com/onuralpszr/supervision.git /tmp/pip-req-build-m2emcu19
  Running command git checkout -b python-toml-file-convert --track origin/python-toml-file-convert
  Switched to a new branch 'python-toml-file-convert'
  Branch 'python-toml-file-convert' set up to track remote branch 'python-toml-file-convert' from 'origin'.
  Resolved https://github.com/onuralpszr/supervision.git to commit f78ac9046400b88c25f240f6aaf1ff0960914196
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Requirement already satisfied: matplotlib<4.0.0,>=3.7.1 in /usr/local/lib/python3.10/dist-packages (from supervision==0.12.0) (3.7.1)
Requirement already satisfied: numpy<2.0.0,>=1.20.0 in /usr/local/lib/python3.10/dist-packages (from supervision==0.12.0) (1.22.4)
Requirement already satisfied: opencv-python-headless<5.0.0.0,>=4.8.0.74 in /usr/local/lib/python3.10/dist-packages (from supervision==0.12.0) (4.8.0.74)
Collecting pillow<11.0.0,>=10.0.0 (from supervision==0.12.0)
  Downloading Pillow-10.0.0-cp310-cp310-manylinux_2_28_x86_64.whl (3.4 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.4/3.4 MB 39.9 MB/s eta 0:00:00
Requirement already satisfied: pyyaml<7.0,>=6.0 in /usr/local/lib/python3.10/dist-packages (from supervision==0.12.0) (6.0)
Requirement already satisfied: contourpy>=1.0.1 in /usr/local/lib/python3.10/dist-packages (from matplotlib<4.0.0,>=3.7.1->supervision==0.12.0) (1.1.0)
Requirement already satisfied: cycler>=0.10 in /usr/local/lib/python3.10/dist-packages (from matplotlib<4.0.0,>=3.7.1->supervision==0.12.0) (0.10.0)
Requirement already satisfied: fonttools>=4.22.0 in /usr/local/lib/python3.10/dist-packages (from matplotlib<4.0.0,>=3.7.1->supervision==0.12.0) (4.41.0)
Requirement already satisfied: kiwisolver>=1.0.1 in /usr/local/lib/python3.10/dist-packages (from matplotlib<4.0.0,>=3.7.1->supervision==0.12.0) (1.4.4)
Requirement already satisfied: packaging>=20.0 in /usr/local/lib/python3.10/dist-packages (from matplotlib<4.0.0,>=3.7.1->supervision==0.12.0) (23.1)
Requirement already satisfied: pyparsing>=2.3.1 in /usr/local/lib/python3.10/dist-packages (from matplotlib<4.0.0,>=3.7.1->supervision==0.12.0) (2.4.7)
Requirement already satisfied: python-dateutil>=2.7 in /usr/local/lib/python3.10/dist-packages (from matplotlib<4.0.0,>=3.7.1->supervision==0.12.0) (2.8.2)
Requirement already satisfied: six in /usr/local/lib/python3.10/dist-packages (from cycler>=0.10->matplotlib<4.0.0,>=3.7.1->supervision==0.12.0) (1.16.0)
Building wheels for collected packages: supervision
  Building wheel for supervision (pyproject.toml) ... done
  Created wheel for supervision: filename=supervision-0.12.0-py3-none-any.whl size=43948 sha256=d958e2d41d129a95bbf1424302046fe106d68706054cf58a54565fa9120a1fae
  Stored in directory: /tmp/pip-ephem-wheel-cache-kd2gbara/wheels/61/e8/3a/b21b55d56dd3341f88c68f8b546a97c0597d2be09e4d82ff19
Successfully built supervision
Installing collected packages: pillow, supervision
  Attempting uninstall: pillow
    Found existing installation: Pillow 8.4.0
    Uninstalling Pillow-8.4.0:
      Successfully uninstalled Pillow-8.4.0
  Attempting uninstall: supervision
    Found existing installation: supervision 0.11.1
    Uninstalling supervision-0.11.1:
      Successfully uninstalled supervision-0.11.1
Successfully installed pillow-10.0.0 supervision-0.12.0
WARNING: The following packages were previously imported in this runtime:
  [PIL]
You must restart the runtime in order to use newly installed versions.

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

onuralpszr · 2023-07-18T13:26:50Z

@SkalskiP it's done. I refresh lock file too.

SkalskiP · 2023-07-18T14:59:02Z

@onuralpszr let me do few tests :)

SkalskiP · 2023-07-18T20:59:48Z

@onuralpszr Finished my tests. Merging. Fingers crossed 🤞🏻

onuralpszr · 2023-07-18T21:00:57Z

@onuralpszr Finished my tests. Merging. Fingers crossed 🤞🏻

Let the fun begin :)

SkalskiP · 2023-07-18T21:13:07Z

Our conversation about Pillow came back to me quickly... It looks like Colab is running the Pillow version with Critical vulnerabilities.

https://github.com/roboflow/supervision/security/dependabot

onuralpszr · 2023-07-18T21:33:21Z

Our conversation about Pillow came back to me quickly... It looks like Colab is running the Pillow version with Critical vulnerabilities.

https://github.com/roboflow/supervision/security/dependabot

Let's open up an issue and keep public ask for users to use "higher" version they just have to restart runtime before they do stuff. At least we are going to be safe zone.

SkalskiP · 2023-07-18T21:52:13Z

@onuralpszr okey, so:

I'll open an issue where we will refer critical Pillow vulnerabilities and start discussing long their plans to address them.
We are keeping ^8.4.0 as our minimal Pillow version requirement for now.
In the above issue, we recommend people to:
```
pip install supervision Pillow==10.0.0
```
If in the notebook, restart the environment.

onuralpszr · 2023-07-18T22:00:23Z

Good idea, and when you install via pip it will already give you latest so we mostly already covered as well.

SkalskiP · 2023-07-18T22:34:01Z

@onuralpszr done: #202

onuralpszr added 3 commits July 5, 2023 00:04

feat(pyproject): 🐍 introduce pyproject toml file

629744d

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

ci(removal): 🧪 removing python 3.7 from test and update other action …

9e1e9fc

…versions Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

fix: toml file version fix and removal of setup.py and adjusment on m…

19271f3

…akefile Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

SkalskiP added the version: 0.12.0 Feature to be added in `0.12.0` release label Jul 4, 2023

ci(pypa-action): introduce pypa github action for uploading to pypi a…

c9d8547

…nd pypi-test Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

onuralpszr mentioned this pull request Jul 4, 2023

WIP - feat: introduce pyproject.toml and pre-commit file #53

Closed

1 task

SkalskiP requested changes Jul 5, 2023

View reviewed changes

onuralpszr added 2 commits July 5, 2023 15:21

docs: 📝 removing python 3.7 metadata pyproject

f7a4a78

docs: 📝 change min python 3.7 to 3.8 in readme Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

feat(version): ✨ dynamic version from __package__

cc81ae7

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

fix: 📝 rename extras group into desktop and headless

c83e57c

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

feat(package): 📦 pillow library added into toml file as deps

a75a927

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

SkalskiP requested changes Jul 7, 2023

View reviewed changes

feat(package): 📦 opencv-python-headless set as default

f78ac90

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

SkalskiP added this to the version: 0.12.0 milestone Jul 18, 2023

SkalskiP self-assigned this Jul 18, 2023

SkalskiP reviewed Jul 18, 2023

View reviewed changes

Comment thread pyproject.toml Outdated

fix: 🐛 lower pillow version for google-colab environment

399121b

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>

SkalskiP approved these changes Jul 18, 2023

View reviewed changes

SkalskiP merged commit 20954a2 into roboflow:main Jul 18, 2023

SkalskiP assigned onuralpszr Jul 21, 2023

ryanjball mentioned this pull request Sep 22, 2023

python version to >=3.8 from 3.6 in setup.py and readme roboflow/roboflow-python#191

Merged

1 task

Conversation

onuralpszr commented Jul 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How has this change been tested, please provide a testcase or example of how you tested the change?

Uh oh!

onuralpszr commented Jul 4, 2023

Uh oh!

SkalskiP commented Jul 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

onuralpszr commented Jul 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

onuralpszr commented Jul 6, 2023

Uh oh!

onuralpszr commented Jul 6, 2023

Uh oh!

SkalskiP commented Jul 7, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

onuralpszr commented Jul 7, 2023

Uh oh!

SkalskiP commented Jul 13, 2023

Uh oh!

onuralpszr commented Jul 13, 2023

Uh oh!

SkalskiP commented Jul 17, 2023

Uh oh!

Uh oh!

onuralpszr commented Jul 18, 2023

Uh oh!

SkalskiP commented Jul 18, 2023

Uh oh!

SkalskiP commented Jul 18, 2023

Uh oh!

onuralpszr commented Jul 18, 2023

Uh oh!

SkalskiP commented Jul 18, 2023

Uh oh!

onuralpszr commented Jul 18, 2023

Uh oh!

SkalskiP commented Jul 18, 2023

Uh oh!

onuralpszr commented Jul 18, 2023

Uh oh!

SkalskiP commented Jul 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

onuralpszr commented Jul 4, 2023 •

edited

Loading

SkalskiP commented Jul 5, 2023 •

edited

Loading

onuralpszr commented Jul 5, 2023 •

edited

Loading