Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ jobs:
run: |
python -m pip install --upgrade pip
pip install pytest pytest-cov
pip install -e ./Python
pip install -e "./Python[experiment]"

- name: Run Python tests
run: |
pytest test_sampling_methods.py test_md5_usage.py test_dl_seeding.py -v --cov=seedhash --cov-report=xml
pytest test_sampling_methods.py test_md5_usage.py test_dl_seeding.py test_correctness.py test_distributed_seeding.py -v --cov=seedhash --cov-report=xml

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v3
Expand All @@ -41,6 +41,30 @@ jobs:
flags: python
name: codecov-python

test-python-torch:
# Runs the multi-process seeding tests (torchrun, several simulated nodes,
# mp.spawn), which are skipped in the matrix above because it has no PyTorch
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.11'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install pytest
pip install torch --index-url https://download.pytorch.org/whl/cpu
pip install -e "./Python[experiment]"

- name: Run PyTorch seeding tests
run: |
pytest test_distributed_seeding.py test_dl_seeding.py -v

test-r:
runs-on: ${{ matrix.os }}
strategy:
Expand All @@ -62,9 +86,15 @@ jobs:
install.packages(c("R6", "digest", "testthat"))
shell: Rscript {0}

- name: Install package
run: |
install.packages("R", repos = NULL, type = "source")
shell: Rscript {0}

- name: Run R tests
run: |
source("R/tests/test_seedhash.R")
testthat::test_dir("R/tests/testthat", package = "seedhash",
load_package = "installed", stop_on_failure = TRUE)
shell: Rscript {0}

lint-python:
Expand Down
25 changes: 14 additions & 11 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,16 @@ We actively welcome pull requests!

3. **Install development dependencies:**
```bash
pip install -e .
pip install -e ".[experiment]"
pip install pytest pytest-cov
```

4. **Run tests:**
4. **Run tests** (the test files are in the repository root):
```bash
pytest tests/ -v
cd ..
pytest test_*.py -v
```
The multi-process tests in `test_distributed_seeding.py` need PyTorch (`pip install torch`) and are skipped without it.

### R Development

Expand Down Expand Up @@ -192,30 +194,31 @@ All contributions must include appropriate tests.
### Python Testing

- Use `pytest` for testing
- Place tests in `Python/tests/`
- Place tests in `test_*.py` files in the repository root
- Aim for high code coverage
- Test edge cases and error conditions

```bash
# Run tests
pytest tests/ -v
# Run tests (from the repository root)
pytest test_*.py -v

# Run with coverage
pytest tests/ --cov=seedhash --cov-report=html
pytest test_*.py --cov=seedhash --cov-report=html
```

### R Testing

- Use `testthat` for testing
- Place tests in `R/tests/`
- Place tests in `R/tests/testthat/`
- Test core functionality and edge cases
- Seed values shared with Python must match the Python tests (see `test_correctness.py`)

```r
# Run tests
devtools::test()
# Run tests (from the repository root)
devtools::test("R")

# Check package
devtools::check()
devtools::check("R")
```

## Documentation
Expand Down
21 changes: 11 additions & 10 deletions GITHUB_INSTALL.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,16 +164,17 @@ library(seedhash)

### ⚠️ Important: R Integer Limitations

R uses 32-bit signed integers with these limits:
- **Minimum**: `-2,147,483,648`
R uses 32-bit signed integers, so `min_value` and `max_value` must be whole numbers between:
- **Minimum**: `-2,147,483,647` (`-2^31` itself cannot be stored as an R integer)
- **Maximum**: `2,147,483,647`
- **Recommended range**: `-1e9` to `1e9` (default)
- **Default range**: `0` to `2^31 - 1`, the same as Python

```r
# ✅ This works
gen <- SeedHashGenerator$new("test", -1e9, 1e9)
# ✅ These work (the full range too)
gen <- SeedHashGenerator$new("test")
gen <- SeedHashGenerator$new("test", -(2^31 - 1), 2^31 - 1)

# ❌ This fails (values too large)
# ❌ This fails (values outside R's integer range)
gen <- SeedHashGenerator$new("test", -10^33, 10^33)
```

Expand All @@ -190,7 +191,7 @@ See [R/INSTALL.md](https://github.com/melhzy/seedhash/tree/main/R/INSTALL.md) fo
| Feature | Python | R |
|---------|--------|---|
| **Integer Range** | Unlimited ✅ | ±2.1 billion ⚠️ |
| **Default Range** | -10^9 to 10^9 | -1e9 to 1e9 |
| **Default Range** | 0 to 2^31 - 1 | 0 to 2^31 - 1 |
| **Dependencies** | None (stdlib only) | R6, digest |
| **Installation** | pip | pak (recommended) |
| **Class System** | Native classes | R6 classes |
Expand Down Expand Up @@ -244,10 +245,10 @@ pip install git+https://github.com/melhzy/seedhash.git#subdirectory=Python
pak::pkg_install("github::melhzy/seedhash/R")
```

### R: "Range is too large"
### R: "outside R's integer range"
```r
# Use smaller range (within ±2.1 billion)
gen <- SeedHashGenerator$new("test", -1e9, 1e9)
# Keep both bounds within ±2,147,483,647
gen <- SeedHashGenerator$new("test", -(2^31 - 1), 2^31 - 1)
```

### R: Package installation corrupted
Expand Down
2 changes: 1 addition & 1 deletion JOSS_CHECKLIST.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ The paper.md includes all required sections:

### Testing ⚠️

- [x] Test files exist (`test_*.py`, `test_seedhash.R`)
- [x] Test files exist (`test_*.py`, `tests/testthat/test-seedhash.R`)
- [x] CI workflow created (`.github/workflows/tests.yml`)
- [ ] **TODO**: Verify CI runs successfully on GitHub
- [ ] **TODO**: Add test coverage badge to README
Expand Down
5 changes: 3 additions & 2 deletions JOSS_PREPARATION_SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,11 @@ I've prepared SeedHash for submission to the Journal of Open Source Software (JO
6. **Verify All Tests Run**
```bash
# Python
pytest test_sampling_methods.py test_md5_usage.py test_dl_seeding.py -v
pytest test_sampling_methods.py test_md5_usage.py test_dl_seeding.py test_correctness.py test_distributed_seeding.py -v

# R
Rscript R/tests/test_seedhash.R
R CMD INSTALL R
Rscript -e 'testthat::test_dir("R/tests/testthat", package = "seedhash", load_package = "installed")'
```

7. **Add Test Coverage Badge** (optional but recommended)
Expand Down
Loading
Loading