Skip to content

Conversation

@plandes
Copy link
Collaborator

@plandes plandes commented Jul 19, 2025

This modernizes the build to pixi and currently provides the following:

  • Run the unit tests in controlled lock (file) environments
  • Build a distribution wheel

@jhnwu3 jhnwu3 requested review from Copilot July 26, 2025 19:53
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 PR modernizes the build system by migrating from Python setuptools to Pixi, a modern cross-platform package manager. It removes legacy setuptools configuration files and introduces comprehensive Pixi configuration for environment management, task execution, and package building.

  • Removes setuptools-based configuration files (setup.py, setup.cfg, requirements-nlp.txt)
  • Introduces comprehensive Pixi configuration with multi-environment support and task definitions
  • Updates test files to use proper unittest structure instead of standalone script execution
  • Modernizes dependency management and CI/CD workflows to use Pixi

Reviewed Changes

Copilot reviewed 16 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pyproject.toml New comprehensive project configuration with Pixi environments and build settings
setup.py, setup.cfg, requirements-nlp.txt Removed legacy setuptools configuration files
tests/todo/test_mortality_prediction.py Converted from script to proper unittest structure
tests/todo/test_mimic_code.py Converted to unittest with commented TODOs for missing functions
tests/core/test_*.py Cleaned up path manipulation imports
pyhealth/tasks/*.py Removed deprecated pkg_resources imports
makefile Updated build system to use Pixi commands
.github/workflows/test.yml Updated CI to use Pixi instead of pip
LICENSE.md Added new BSD-3-Clause license file
Comments suppressed due to low confidence (10)

pyproject.toml:23

  • Python 3.13 does not exist yet. The latest stable Python version is 3.12. Consider changing to ">=3.10,<3.13" or similar.
requires-python = ">=3.13,<3.14"

pyproject.toml:25

  • PyTorch 2.7.1 does not exist. The latest PyTorch version is around 2.1.x. Consider using "torch~=2.1.0" or remove the specific patch version.
  "torch~=2.7.1",

pyproject.toml:28

  • RDKit 2025.3.3 is a future version that does not exist. Consider using a current stable version like "rdkit~=2023.9.0".
  "rdkit==2025.3.3",

pyproject.toml:98

  • Python 3.13 does not exist yet. This should match the requires-python field and use an existing Python version.
python = "~=3.13"

pyproject.toml:126

  • Python 3.13 does not exist yet. This should use an existing Python version that matches the project requirements.
python = "~=3.13"

.github/workflows/test.yml:24

  • Python 3.13 does not exist yet. Use an existing Python version like '3.12' or '3.11'.
          python-version: '3.13'

tests/todo/test_mimic_code.py:104

  • The class name 'Test' is too generic. Consider a more descriptive name like 'TestMimicCode' or 'MimicCodeTests'.
class Test(unittest.TestCase):

tests/todo/test_mortality_prediction.py:418

  • The class name 'Test' is too generic. Consider a more descriptive name like 'TestMortalityPrediction' or 'MortalityPredictionTests'.
class Test(unittest.TestCase):

tests/todo/test_mortality_prediction.py:419

  • The method name 'test' is too generic. Consider more descriptive names like 'test_mimic3_mortality_prediction' and 'test_multimodal_mortality_prediction'.
    def test(self):

tests/todo/test_mimic_code.py:105

  • The method name 'test' is too generic. Consider a more descriptive name like 'test_mimic_code_functionality'.
    def test(self):

try:
pkg_resources.require(dependencies)
from importlib.metadata import version
version(dependencies)
Copy link

Copilot AI Jul 26, 2025

Choose a reason for hiding this comment

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

The variable dependencies is a list but version() expects a string package name. This should be version(dependencies[0]) or loop through the dependencies list.

Suggested change
version(dependencies)
version(dependencies[0])

Copilot uses AI. Check for mistakes.
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Collaborator

@jhnwu3 jhnwu3 left a comment

Choose a reason for hiding this comment

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

Works! Will need to update the metadata to PyHealth 2.0a, but let's keep this rolling!

@jhnwu3 jhnwu3 merged commit 6231323 into sunlabuiuc:master Jul 26, 2025
1 check passed
dalloliogm pushed a commit to dalloliogm/PyHealth that referenced this pull request Nov 26, 2025
* replace setuptools build with pixi

* fix unit tests

* add test.py to pixi tests; move remaining unittests out of src

* convert CLI run tests to Python unittest; rolled back to todo
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.

2 participants