Skip to content

Conversation

@glistening
Copy link
Contributor

It introduces opm tools for building package from pytorch.

ONE-DCO-1.0-Signed-off-by: Sanggyu Lee sg5.lee@samsung.com

@hseok-oh
Copy link
Contributor

hseok-oh commented Dec 8, 2025

Please use pyproject.toml instead of requirements.txt

print(f"Downloading model files for {model_id} into {target_dir}...")

# Patch httpx to disable SSL verification and forward proxy if set
import httpx
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you put 'import' to top of the script?

# Patch httpx to disable SSL verification and forward proxy if set
import httpx
original_client = httpx.Client
proxy = os.getenv("HTTPS_PROXY") or os.getenv("HTTP_PROXY")
Copy link
Contributor

Choose a reason for hiding this comment

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

Proxy settings can also work with lower case too, (e.g. 'http_proxy').
I explored a more generic approach and found one that utilizes 'urllib.request'.

import urllib.request
proxy = urllib.request.getproxies()

It works in a case-insensitive way.

# PR-related constants (used only in init.py)
PR_WORKTREE = "_pr_16233"
PR_BRANCH = "pr-16233"
PR_REF = "refs/pull/16233/head"
Copy link
Contributor

Choose a reason for hiding this comment

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

@glistening
Copy link
Contributor Author

glistening commented Dec 9, 2025

Please use pyproject.toml instead of requirements.txt

@hseok-oh Thank you for review! 👍

It would be good to use pyproject.toml. However, I can't at this moment because of TICO. It installs a ton of packages which is not necessary at all (especially related to nvidia-*) As you can see the my init.py it workarounds by instaling torch-cpu first before installing TICO's huge dependencies. If I use pyproject.toml, there seems no way to avoid the huge download and installing overhead (time and energy).

It would be best to fix the requirements of TICO. However, I guess it would not be easy thing.

Anyway, if we don't care about spending time and money, I would update init.py (including pyproject.toml).

(ADD) I introduced pyproject.toml w/o TICO, which is installed in init.py.

@glistening
Copy link
Contributor Author

glistening commented Dec 9, 2025

@batcheu Thank you for review! I never thought it is not recommended way in Python.

I updated as you commented. I am not sure PEP-8 is happy about the code.
It would be safe to use some python format tool.
yapf ( ← ./nnas format ) seems not check at this level.

It introduces opm tools for building package from pytorch.

ONE-DCO-1.0-Signed-off-by: Sanggyu Lee <sg5.lee@samsung.com>
@glistening glistening added the PR/NO MERGE Please don't merge. I'm still working on this :) label Dec 9, 2025
@glistening
Copy link
Contributor Author

glistening commented Dec 9, 2025

I added PR/NO MERGE since it needs to check:

  • Re-check Python Coding Convention (PEP-8 + more ( ? ) as @batcheu suggested)
    • TICO seems to check more as I remember
  • Works in Suwon
  • ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR/NO MERGE Please don't merge. I'm still working on this :)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants