-
Notifications
You must be signed in to change notification settings - Fork 220
[feat]: added one-logger support for NeMo RL #1631
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,3 +18,6 @@ | |||||||||||||||||
| url = https://github.com/NVIDIA-NeMo/Gym.git | ||||||||||||||||||
| branch = main | ||||||||||||||||||
| shallow = true | ||||||||||||||||||
| [submodule "3rdparty/NV-One-Logger-workspace/nv-one-logger"] | ||||||||||||||||||
| path = 3rdparty/NV-One-Logger-workspace/nv-one-logger | ||||||||||||||||||
| url = [email protected]:NVIDIA/nv-one-logger.git | ||||||||||||||||||
|
Comment on lines
+21
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chain🏁 Script executed: cat -n .gitmodulesRepository: NVIDIA-NeMo/RL Length of output: 1102 🏁 Script executed: curl -s -o /dev/null -w "%{http_code}" https://github.com/NVIDIA/nv-one-loggerRepository: NVIDIA-NeMo/RL Length of output: 61 Add branch and shallow clone configuration for consistency. The new submodule entry lacks
Apply this diff to align with existing submodule configurations: [submodule "3rdparty/NV-One-Logger-workspace/nv-one-logger"]
path = 3rdparty/NV-One-Logger-workspace/nv-one-logger
- url = [email protected]:NVIDIA/nv-one-logger.git
+ url = https://github.com/NVIDIA/nv-one-logger.git
+ branch = main
+ shallow = true📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # Copyright (c) 2025, NVIDIA CORPORATION. All rights reserved. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| try: | ||
| import nv_one_logger # noqa: F401 | ||
|
|
||
| INSTALLED = True | ||
| except ImportError: | ||
| INSTALLED = False | ||
|
|
||
| print(f"NV One Logger {INSTALLED=}") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| [build-system] | ||
| requires = ["setuptools", "wheel", "toml"] | ||
| build-backend = "setuptools.build_meta" | ||
|
|
||
| [project] | ||
| name = "nv-one-logger-core" | ||
| description = "Wrapper for nv-one-logger-core" | ||
| requires-python = ">=3.8" | ||
| dynamic = ["version", "dependencies"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import sys | ||
| import toml | ||
| from pathlib import Path | ||
| import setuptools | ||
|
Comment on lines
+1
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Missing NVIDIA copyright header and unused import. Same issues as the wrapper setup.py files: add the NVIDIA copyright header and remove the unused Apply this diff: +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
-import sys
import toml
from pathlib import Path
import setuptools🤖 Prompt for AI Agents |
||
|
|
||
| # Define the source directory for the actual package (git-cloned repo) | ||
| src_dir = Path(__file__).parent / "nv-one-logger" / "nv_one_logger" / "one_logger_core" | ||
|
|
||
| if not src_dir.exists(): | ||
| raise FileNotFoundError(f"{src_dir} not found.") | ||
|
|
||
| # Read the underlying pyproject.toml | ||
| pyproject_toml_path = src_dir / "pyproject.toml" | ||
| with pyproject_toml_path.open("r") as f: | ||
| pyproject_data = toml.load(f) | ||
|
|
||
| # Extract dependencies | ||
| # Handle poetry dependencies format | ||
| poetry_deps = pyproject_data.get("tool", {}).get("poetry", {}).get("dependencies", {}) | ||
| install_requires = [] | ||
| for pkg, ver in poetry_deps.items(): | ||
| if pkg == "python": | ||
| continue | ||
| # Simple conversion, might need more robust handling for complex version strings | ||
| # but sufficient for this specific repo's needs | ||
| if isinstance(ver, str): | ||
| if ver.startswith("^"): | ||
| install_requires.append(f"{pkg}>={ver[1:]}") | ||
| else: | ||
| install_requires.append(f"{pkg}=={ver}") | ||
| elif isinstance(ver, dict): | ||
| # Handle cases like {version = "...", markers = "..."} if present | ||
| if "version" in ver: | ||
| version_spec = ver['version'] | ||
| # If version already has operators (>=, ==, etc.), use as-is | ||
| if any(op in version_spec for op in ['>=', '<=', '==', '!=', '~=', '>', '<']): | ||
| install_requires.append(f"{pkg}{version_spec}") | ||
| else: | ||
| install_requires.append(f"{pkg}>={version_spec}") | ||
|
|
||
| # Extract version | ||
| version = pyproject_data.get("tool", {}).get("poetry", {}).get("version", "0.0.0") | ||
|
|
||
| # Map the package directory | ||
| # The package name in source is 'nv_one_logger' (from src directory in one_logger_core) | ||
| # one_logger_core/src/nv_one_logger | ||
| package_dir = { | ||
| "": str(src_dir / "src") | ||
| } | ||
| packages = setuptools.find_packages(where=str(src_dir / "src")) | ||
|
|
||
| setuptools.setup( | ||
| name="nv-one-logger-core", | ||
| version=version, | ||
| description="Wrapper for nv-one-logger-core", | ||
| packages=packages, | ||
| package_dir=package_dir, | ||
| install_requires=install_requires, | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so for all the one logger dependencies, can we switch to VCS installs in pyproject.toml? see https://github.com/NVIDIA-NeMo/RL/blob/main/pyproject.toml#L154 for example for context, the 3rdparty repo contains dependencies we expect to change a lot and require us to edit the code to have a monorepo experience. considering logging is probably not something we will change very often, i think it will be easier to maintain this as a regular python package. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||
| [build-system] | ||||||
| requires = ["setuptools", "wheel", "toml"] | ||||||
| build-backend = "setuptools.build_meta" | ||||||
|
|
||||||
| [project] | ||||||
| name = "nv-one-logger-otel" | ||||||
| description = "Wrapper for nv-one-logger-otel" | ||||||
| requires-python = ">=3.8" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent Python version requirement. The Apply this diff to align with project standards: -requires-python = ">=3.8"
+requires-python = ">=3.12"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| dynamic = ["version", "dependencies"] | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,59 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||||||||||||||||||
| import toml | ||||||||||||||||||||||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||||||||||||||||||||||
| import setuptools | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Missing NVIDIA copyright header and unused import. Per coding guidelines, Python files require the NVIDIA copyright header. Also, Apply this diff: +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
-import sys
import toml
from pathlib import Path
import setuptools📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Define the source directory for the actual package (git-cloned repo) | ||||||||||||||||||||||||||||||||||||||||||||||
| src_dir = Path(__file__).parent.parent.parent / "nv-one-logger" / "nv_one_logger" / "one_logger_otel" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if not src_dir.exists(): | ||||||||||||||||||||||||||||||||||||||||||||||
| raise FileNotFoundError(f"{src_dir} not found.") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Read the underlying pyproject.toml | ||||||||||||||||||||||||||||||||||||||||||||||
| pyproject_toml_path = src_dir / "pyproject.toml" | ||||||||||||||||||||||||||||||||||||||||||||||
| with pyproject_toml_path.open("r") as f: | ||||||||||||||||||||||||||||||||||||||||||||||
| pyproject_data = toml.load(f) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Extract dependencies | ||||||||||||||||||||||||||||||||||||||||||||||
| # Handle poetry dependencies format | ||||||||||||||||||||||||||||||||||||||||||||||
| poetry_deps = pyproject_data.get("tool", {}).get("poetry", {}).get("dependencies", {}) | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires = [] | ||||||||||||||||||||||||||||||||||||||||||||||
| for pkg, ver in poetry_deps.items(): | ||||||||||||||||||||||||||||||||||||||||||||||
| if pkg == "python": | ||||||||||||||||||||||||||||||||||||||||||||||
| continue | ||||||||||||||||||||||||||||||||||||||||||||||
| # Simple conversion, might need more robust handling for complex version strings | ||||||||||||||||||||||||||||||||||||||||||||||
| # but sufficient for this specific repo's needs | ||||||||||||||||||||||||||||||||||||||||||||||
| if isinstance(ver, str): | ||||||||||||||||||||||||||||||||||||||||||||||
| if ver.startswith("^"): | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}>={ver[1:]}") | ||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}=={ver}") | ||||||||||||||||||||||||||||||||||||||||||||||
| elif isinstance(ver, dict): | ||||||||||||||||||||||||||||||||||||||||||||||
| # Handle cases like {version = "...", markers = "..."} if present | ||||||||||||||||||||||||||||||||||||||||||||||
| if "version" in ver: | ||||||||||||||||||||||||||||||||||||||||||||||
| version_spec = ver['version'] | ||||||||||||||||||||||||||||||||||||||||||||||
| # If version already has operators (>=, ==, etc.), use as-is | ||||||||||||||||||||||||||||||||||||||||||||||
| if any(op in version_spec for op in ['>=', '<=', '==', '!=', '~=', '>', '<']): | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}{version_spec}") | ||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}>={version_spec}") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Extract version | ||||||||||||||||||||||||||||||||||||||||||||||
| version = pyproject_data.get("tool", {}).get("poetry", {}).get("version", "0.0.0") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Map the package directory | ||||||||||||||||||||||||||||||||||||||||||||||
| # The package name in source is 'nv_one_logger' (from src directory in one_logger_otel) | ||||||||||||||||||||||||||||||||||||||||||||||
| # one_logger_otel/src/nv_one_logger | ||||||||||||||||||||||||||||||||||||||||||||||
| package_dir = { | ||||||||||||||||||||||||||||||||||||||||||||||
| "": str(src_dir / "src") | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| packages = setuptools.find_packages(where=str(src_dir / "src")) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| setuptools.setup( | ||||||||||||||||||||||||||||||||||||||||||||||
| name="nv-one-logger-otel", | ||||||||||||||||||||||||||||||||||||||||||||||
| version=version, | ||||||||||||||||||||||||||||||||||||||||||||||
| description="Extensions to onelogger library to use Open telemetry (OTEL) as a backend.", | ||||||||||||||||||||||||||||||||||||||||||||||
| packages=packages, | ||||||||||||||||||||||||||||||||||||||||||||||
| package_dir=package_dir, | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires=install_requires, | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||
| [build-system] | ||||||
| requires = ["setuptools", "wheel", "toml"] | ||||||
| build-backend = "setuptools.build_meta" | ||||||
|
|
||||||
| [project] | ||||||
| name = "nv-one-logger-training-telemetry" | ||||||
| description = "Wrapper for nv-one-logger-training-telemetry" | ||||||
| requires-python = ">=3.8" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent Python version requirement. The Apply this diff to align with project standards: -requires-python = ">=3.8"
+requires-python = ">=3.12"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| dynamic = ["version", "dependencies"] | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,59 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||||||||||||||||||
| import toml | ||||||||||||||||||||||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||||||||||||||||||||||
| import setuptools | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Missing NVIDIA copyright header and unused import. Same issues as other wrappers: add the NVIDIA copyright header and remove the unused Apply this diff: +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
-import sys
import toml
from pathlib import Path
import setuptools📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Define the source directory for the actual package (git-cloned repo) | ||||||||||||||||||||||||||||||||||||||||||||||
| src_dir = Path(__file__).parent.parent.parent / "nv-one-logger" / "nv_one_logger" / "one_logger_training_telemetry" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if not src_dir.exists(): | ||||||||||||||||||||||||||||||||||||||||||||||
| raise FileNotFoundError(f"{src_dir} not found.") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Read the underlying pyproject.toml | ||||||||||||||||||||||||||||||||||||||||||||||
| pyproject_toml_path = src_dir / "pyproject.toml" | ||||||||||||||||||||||||||||||||||||||||||||||
| with pyproject_toml_path.open("r") as f: | ||||||||||||||||||||||||||||||||||||||||||||||
| pyproject_data = toml.load(f) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Extract dependencies | ||||||||||||||||||||||||||||||||||||||||||||||
| # Handle poetry dependencies format | ||||||||||||||||||||||||||||||||||||||||||||||
| poetry_deps = pyproject_data.get("tool", {}).get("poetry", {}).get("dependencies", {}) | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires = [] | ||||||||||||||||||||||||||||||||||||||||||||||
| for pkg, ver in poetry_deps.items(): | ||||||||||||||||||||||||||||||||||||||||||||||
| if pkg == "python": | ||||||||||||||||||||||||||||||||||||||||||||||
| continue | ||||||||||||||||||||||||||||||||||||||||||||||
| # Simple conversion, might need more robust handling for complex version strings | ||||||||||||||||||||||||||||||||||||||||||||||
| # but sufficient for this specific repo's needs | ||||||||||||||||||||||||||||||||||||||||||||||
| if isinstance(ver, str): | ||||||||||||||||||||||||||||||||||||||||||||||
| if ver.startswith("^"): | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}>={ver[1:]}") | ||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}=={ver}") | ||||||||||||||||||||||||||||||||||||||||||||||
| elif isinstance(ver, dict): | ||||||||||||||||||||||||||||||||||||||||||||||
| # Handle cases like {version = "...", markers = "..."} if present | ||||||||||||||||||||||||||||||||||||||||||||||
| if "version" in ver: | ||||||||||||||||||||||||||||||||||||||||||||||
| version_spec = ver['version'] | ||||||||||||||||||||||||||||||||||||||||||||||
| # If version already has operators (>=, ==, etc.), use as-is | ||||||||||||||||||||||||||||||||||||||||||||||
| if any(op in version_spec for op in ['>=', '<=', '==', '!=', '~=', '>', '<']): | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}{version_spec}") | ||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}>={version_spec}") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Extract version | ||||||||||||||||||||||||||||||||||||||||||||||
| version = pyproject_data.get("tool", {}).get("poetry", {}).get("version", "0.0.0") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Map the package directory | ||||||||||||||||||||||||||||||||||||||||||||||
| # The package name in source is 'nv_one_logger' (from src directory in one_logger_training_telemetry) | ||||||||||||||||||||||||||||||||||||||||||||||
| # one_logger_training_telemetry/src/nv_one_logger | ||||||||||||||||||||||||||||||||||||||||||||||
| package_dir = { | ||||||||||||||||||||||||||||||||||||||||||||||
| "": str(src_dir / "src") | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| packages = setuptools.find_packages(where=str(src_dir / "src")) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| setuptools.setup( | ||||||||||||||||||||||||||||||||||||||||||||||
| name="nv-one-logger-training-telemetry", | ||||||||||||||||||||||||||||||||||||||||||||||
| version=version, | ||||||||||||||||||||||||||||||||||||||||||||||
| description="Training job telemetry using OneLogger library.", | ||||||||||||||||||||||||||||||||||||||||||||||
| packages=packages, | ||||||||||||||||||||||||||||||||||||||||||||||
| package_dir=package_dir, | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires=install_requires, | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||
| [build-system] | ||||||
| requires = ["setuptools", "wheel", "toml"] | ||||||
| build-backend = "setuptools.build_meta" | ||||||
|
|
||||||
| [project] | ||||||
| name = "nv-one-logger-wandb" | ||||||
| description = "Wrapper for nv-one-logger-wandb" | ||||||
| requires-python = ">=3.9" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent Python version requirement. The Apply this diff to align with project standards: -requires-python = ">=3.9"
+requires-python = ">=3.12"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| dynamic = ["version", "dependencies"] | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,59 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||||||||||||||||||
| import toml | ||||||||||||||||||||||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||||||||||||||||||||||
| import setuptools | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Missing NVIDIA copyright header and unused import. Same issues as the otel wrapper: add the NVIDIA copyright header and remove the unused Apply this diff: +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
-import sys
import toml
from pathlib import Path
import setuptools📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Define the source directory for the actual package (git-cloned repo) | ||||||||||||||||||||||||||||||||||||||||||||||
| src_dir = Path(__file__).parent.parent.parent / "nv-one-logger" / "nv_one_logger" / "one_logger_wandb" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if not src_dir.exists(): | ||||||||||||||||||||||||||||||||||||||||||||||
| raise FileNotFoundError(f"{src_dir} not found.") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Read the underlying pyproject.toml | ||||||||||||||||||||||||||||||||||||||||||||||
| pyproject_toml_path = src_dir / "pyproject.toml" | ||||||||||||||||||||||||||||||||||||||||||||||
| with pyproject_toml_path.open("r") as f: | ||||||||||||||||||||||||||||||||||||||||||||||
| pyproject_data = toml.load(f) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Extract dependencies | ||||||||||||||||||||||||||||||||||||||||||||||
| # Handle poetry dependencies format | ||||||||||||||||||||||||||||||||||||||||||||||
| poetry_deps = pyproject_data.get("tool", {}).get("poetry", {}).get("dependencies", {}) | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires = [] | ||||||||||||||||||||||||||||||||||||||||||||||
| for pkg, ver in poetry_deps.items(): | ||||||||||||||||||||||||||||||||||||||||||||||
| if pkg == "python": | ||||||||||||||||||||||||||||||||||||||||||||||
| continue | ||||||||||||||||||||||||||||||||||||||||||||||
| # Simple conversion, might need more robust handling for complex version strings | ||||||||||||||||||||||||||||||||||||||||||||||
| # but sufficient for this specific repo's needs | ||||||||||||||||||||||||||||||||||||||||||||||
| if isinstance(ver, str): | ||||||||||||||||||||||||||||||||||||||||||||||
| if ver.startswith("^"): | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}>={ver[1:]}") | ||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}=={ver}") | ||||||||||||||||||||||||||||||||||||||||||||||
| elif isinstance(ver, dict): | ||||||||||||||||||||||||||||||||||||||||||||||
| # Handle cases like {version = "...", markers = "..."} if present | ||||||||||||||||||||||||||||||||||||||||||||||
| if "version" in ver: | ||||||||||||||||||||||||||||||||||||||||||||||
| version_spec = ver['version'] | ||||||||||||||||||||||||||||||||||||||||||||||
| # If version already has operators (>=, ==, etc.), use as-is | ||||||||||||||||||||||||||||||||||||||||||||||
| if any(op in version_spec for op in ['>=', '<=', '==', '!=', '~=', '>', '<']): | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}{version_spec}") | ||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires.append(f"{pkg}>={version_spec}") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Extract version | ||||||||||||||||||||||||||||||||||||||||||||||
| version = pyproject_data.get("tool", {}).get("poetry", {}).get("version", "0.0.0") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Map the package directory | ||||||||||||||||||||||||||||||||||||||||||||||
| # The package name in source is 'nv_one_logger' (from src directory in one_logger_wandb) | ||||||||||||||||||||||||||||||||||||||||||||||
| # one_logger_wandb/src/nv_one_logger | ||||||||||||||||||||||||||||||||||||||||||||||
| package_dir = { | ||||||||||||||||||||||||||||||||||||||||||||||
| "": str(src_dir / "src") | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| packages = setuptools.find_packages(where=str(src_dir / "src")) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| setuptools.setup( | ||||||||||||||||||||||||||||||||||||||||||||||
| name="nv-one-logger-wandb", | ||||||||||||||||||||||||||||||||||||||||||||||
| version=version, | ||||||||||||||||||||||||||||||||||||||||||||||
| description="Extensions to onelogger library to use Weights and Biases as a backend.", | ||||||||||||||||||||||||||||||||||||||||||||||
| packages=packages, | ||||||||||||||||||||||||||||||||||||||||||||||
| package_dir=package_dir, | ||||||||||||||||||||||||||||||||||||||||||||||
| install_requires=install_requires, | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't be needed