Skip to content

UV package integration for python#65

Closed
kamalcou wants to merge 5 commits intoCIROH-UA:mainfrom
kamalcou:main
Closed

UV package integration for python#65
kamalcou wants to merge 5 commits intoCIROH-UA:mainfrom
kamalcou:main

Conversation

@kamalcou
Copy link
Collaborator

[Short description explaining the high-level reason for the pull request]

Additions

requirements.txt
readme.md -- uv installation
.gitignore -- add !requirements.txt
pyproject.toml-- modified
uv.lock
configs/conf_fp.json -- add relative path

Removals

Changes

Testing

Screenshot 2026-02-20 at 2 56 57 PM

Screenshots

Notes

Todos

Checklist

  • PR has an informative and human-readable title
  • [x ] Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Windows
  • Linux
  • Browser

Accessibility

  • Keyboard friendly
  • Screen reader friendly

Other

  • Is useable without CSS
  • Is useable without JS
  • Flexible from small to large screens
  • No linting errors or warnings
  • JavaScript tests are passing

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request migrates the forcingprocessor project from traditional setuptools-based dependency management to UV, a modern Python package manager. The changes introduce UV-specific configuration files and update the installation instructions to use UV commands.

Changes:

  • Migration from setuptools to UV build backend with new pyproject.toml configuration
  • Addition of requirements.txt and .python-version files for UV compatibility
  • Updated README.md with UV installation and usage instructions
  • Updated example configuration file with relative paths for easier testing

Reviewed changes

Copilot reviewed 4 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
pyproject.toml Migrated from setuptools to UV build backend, added project metadata and pinned all dependencies
requirements.txt Added comprehensive list of pinned dependencies (87 packages)
README.md Updated installation instructions to use UV package manager
configs/conf_fp.json Changed paths from absolute to relative for example data files
.python-version Specified Python 3.11 as the project Python version
.gitignore Added data/forcing/* to ignore output directories

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

]

[project.scripts]
forcingprocessor = "forcingprocessor:main"
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The project.scripts entry "forcingprocessor = 'forcingprocessor:main'" references a 'main' function in the forcingprocessor module, but this function does not appear to exist in init.py. You need to either define a main() function in init.py or update the entry point to reference the correct module and function (e.g., "forcingprocessor.processor:main" if there's a main function in processor.py).

Suggested change
forcingprocessor = "forcingprocessor:main"
forcingprocessor = "forcingprocessor.processor:main"

Copilot uses AI. Check for mistakes.

## Install
From root
## Install UV early
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The heading "Install UV early" contains a grammatical error. It should be "Install UV First" or "Install UV" for better clarity and professional tone.

Suggested change
## Install UV early
## Install UV

Copilot uses AI. Check for mistakes.
Comment on lines +10 to +98
dependencies = [
"affine==2.4.0",
"aiohappyeyeballs==2.6.1",
"aiohttp==3.13.3",
"aiosignal==1.4.0",
"attrs==25.4.0",
"boto3==1.42.49",
"botocore==1.42.49",
"certifi==2026.1.4",
"cffi==2.0.0",
"cftime==1.6.5",
"charset-normalizer==3.4.4",
"click==8.3.1",
"click-plugins==1.1.1.2",
"cligj==0.7.2",
"cloudpickle==3.1.2",
"contourpy==1.3.3",
"cryptography==46.0.5",
"cycler==0.12.1",
"dask==2026.1.2",
"decorator==5.2.1",
"exactextract==0.3.0",
"fonttools==4.61.1",
"frozenlist==1.8.0",
"fsspec==2026.2.0",
"gcsfs==2026.2.0",
"geopandas==1.1.2",
"google-api-core==2.29.0",
"google-auth==2.48.0",
"google-auth-oauthlib==1.2.4",
"google-cloud-core==2.5.0",
"google-cloud-storage==3.9.0",
"google-cloud-storage-control==1.10.0",
"google-crc32c==1.8.0",
"google-resumable-media==2.8.0",
"googleapis-common-protos==1.72.0",
"grpc-google-iam-v1==0.14.3",
"grpcio==1.78.0",
"grpcio-status==1.78.0",
"h5netcdf==1.8.1",
"h5py==3.15.1",
"idna==3.11",
"imageio==2.37.2",
"importlib-metadata==8.7.1",
"jmespath==1.1.0",
"kiwisolver==1.4.9",
"locket==1.0.0",
"matplotlib==3.10.8",
"multidict==6.7.1",
"netcdf4==1.7.4",
"numpy==2.4.2",
"nwmurl==1.0.1",
"oauthlib==3.3.1",
"packaging==26.0",
"pandas==3.0.0",
"partd==1.4.2",
"pillow==12.1.1",
"propcache==0.4.1",
"proto-plus==1.27.1",
"protobuf==6.33.5",
"psutil==7.2.2",
"pyarrow==23.0.0",
"pyasn1==0.6.2",
"pyasn1-modules==0.4.2",
"pycparser==3.0",
"pyogrio==0.12.1",
"pyparsing==3.3.2",
"pyproj==3.7.2",
"pysam==0.23.3",
"python-dateutil==2.9.0.post0",
"pyyaml==6.0.3",
"rasterio==1.4.4",
"requests==2.32.5",
"requests-oauthlib==2.0.0",
"rioxarray==0.19.0",
"rsa==4.9.1",
"rust==1.3.1",
"s3fs==0.4.2",
"s3transfer==0.16.0",
"scipy==1.17.0",
"shapely==2.1.2",
"six==1.17.0",
"toolz==1.1.0",
"typing-extensions==4.15.0",
"urllib3==2.6.3",
"xarray==2026.1.0",
"yarl==1.22.0",
"zipp==3.23.0",
]
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The migration from flexible version constraints (e.g., "pandas" without version) in setup.cfg to pinned exact versions (e.g., "pandas==3.0.0") represents a significant change in dependency management philosophy. While this provides reproducibility, it may cause compatibility issues and makes maintenance harder. Consider using version ranges (e.g., "pandas>=2.0,<4.0") instead of exact pins for better compatibility and easier updates, especially for a library package.

Copilot uses AI. Check for mistakes.
pip install -e .
curl -LsSf https://astral.sh/uv/install.sh | sh
```
## install requirement files
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The heading "install requirement files" should be capitalized consistently with other section headings. It should be "Install Requirement Files" or "Install Requirements".

Suggested change
## install requirement files
## Install Requirement Files

Copilot uses AI. Check for mistakes.
@@ -1,6 +1,105 @@
[project]
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The PR description lists "uv.lock" as an addition, but this file is not included in the pull request diff. The uv.lock file is critical for reproducible builds and should be included when migrating to UV package management. Please ensure uv.lock is properly added to the repository.

Copilot uses AI. Check for mistakes.
Comment on lines +104 to +105
requires = ["uv_build>=0.9.28,<0.10.0"]
build-backend = "uv_build"
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The build backend "uv_build" specified here may not be the correct package name. The standard UV build backend should be specified as "hatchling" or another established build backend, as UV itself is a package manager and installer, not a build backend. Please verify the correct build backend configuration for UV-based projects.

Suggested change
requires = ["uv_build>=0.9.28,<0.10.0"]
build-backend = "uv_build"
requires = ["hatchling"]
build-backend = "hatchling"

Copilot uses AI. Check for mistakes.
description = "Tool to convert nwm forcing netcdfs to ngen compatible files"
readme ="README.md"
authors = [
{ name = "Jordan J. Laser", email = " [email protected]" }
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

There is an extra space in the email address " [email protected]" (space before 'jlaser'). This should be corrected to "[email protected]" without the leading space.

Suggested change
{ name = "Jordan J. Laser", email = " [email protected]" }
{ name = "Jordan J. Laser", email = "[email protected]" }

Copilot uses AI. Check for mistakes.
python src/forcingprocessor/processor.py ./configs/conf.json
uv add --dev -r requirements.txt
```
## make directory for output data
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The heading "make directory for output data" should be capitalized consistently with other section headings. It should be "Make Directory for Output Data" or "Create Output Directory".

Suggested change
## make directory for output data
## Make Directory for Output Data

Copilot uses AI. Check for mistakes.
authors = [
{ name = "Jordan J. Laser", email = " [email protected]" }
]
requires-python = ">=3.11"
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The Python version requirement changed from ">=3.9" (in setup.cfg) to ">=3.11" (in pyproject.toml). This is a breaking change that should be documented in the PR description or CHANGELOG, as it may affect users running Python 3.9 or 3.10. Please ensure this change is intentional and properly communicated.

Suggested change
requires-python = ">=3.11"
requires-python = ">=3.9"

Copilot uses AI. Check for mistakes.
README.md Outdated
## Run the forcingprocessor
```
python src/forcingprocessor/processor.py ./configs/conf.json
uv add --dev -r requirements.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

This installs all the dependencies, which is nice and makes the following command uv run python src/forcingprocessor/processor.py ./configs/conf_fp.json work fine, but if the purpose of this pull request was to make uv install forcingprocessor as a package, I don't think this command does that. However, I am not sure what the purpose of this PR was or what issue it closes, so this comment is just a comment and not a criticism

pyparsing==3.3.2
pyproj==3.7.2
pysam==0.23.3
python-dateutil==2.9.0.post0
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice to have pytest in here or the pyproject.toml, whichever one you end up using to install dependencies or build the forcingprocessor package

Co-authored-by: Copilot <[email protected]>
Copy link
Collaborator Author

@kamalcou kamalcou left a comment

Choose a reason for hiding this comment

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

Added uv pip install -r requirements.txt instead of uv add --dev -r requirements.txt

@kamalcou
Copy link
Collaborator Author

Added pytest==9.0.2 in requirements.txt

@JordanLaserGit JordanLaserGit requested review from quinnylee and removed request for quinnylee March 10, 2026 18:08
Copy link
Collaborator

@JordanLaserGit JordanLaserGit left a comment

Choose a reason for hiding this comment

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

PRs to this repository must be done from a CIROH-UA/forcingprocessor branch so that gitactions will run. It's possible to enable the actions in a forked branch, but the workflows require secrets that are not available to forked branch actions. Please create a branch from CIROH-UA/forcingprocessor and make another PR.

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.

4 participants