Skip to content

Commit

Permalink
Separate Hashing from Persist-Filepath Function (#19)
Browse files Browse the repository at this point in the history
* fix now broken flake8 link

* pre-commit hook updates beyond fixing flake8 repo

* version bump

* separate hash property

* hash repeatability test

* version bump

* Simplify test matrix
  • Loading branch information
aloosley authored Dec 17, 2024
1 parent 0087f6e commit 0da5c41
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 18 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/python-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [macos-latest, windows-latest, ubuntu-18.04, ubuntu-20.04]
python: [3.9.13, 3.10.4]
os: [windows-latest, ubuntu-latest]
python: [3.11.11, 3.13.0]
steps:
- name: Checkout
uses: actions/checkout@v3
Expand Down
10 changes: 5 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ default_stages: [commit]
fail_fast: true
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.3.0
rev: v5.0.0
hooks:
- id: check-yaml
- id: check-json
Expand All @@ -15,15 +15,15 @@ repos:
files: ^.ipynb
- id: trailing-whitespace
- repo: https://github.com/psf/black
rev: 22.3.0
rev: 24.10.0
hooks:
- id: black
- repo: https://gitlab.com/pycqa/flake8
rev: 4.0.1
- repo: https://github.com/pycqa/flake8
rev: 7.1.1
hooks:
- id: flake8
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.961
rev: v1.13.0
hooks:
- id: mypy
name: mypy --strict
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
![](https://img.shields.io/badge/version-1.2.2-green.svg)
![](https://img.shields.io/badge/version-1.2.3-green.svg)
[![Build & Test](https://github.com/aloosley/persistable/actions/workflows/python-build.yml/badge.svg)](https://github.com/aloosley/persistable/actions/workflows/python-build.yml)
[![contributions welcome](https://img.shields.io/badge/contributions-welcome-brightgreen.svg?style=flat)](https://github.com/dwyl/esta/issues)

Expand Down
2 changes: 1 addition & 1 deletion persistable/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .base import Persistable # noqa
from .data import PersistableParams # noqa

__version__ = "1.2.2"
__version__ = "1.2.3"
8 changes: 5 additions & 3 deletions persistable/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ def generate(self, persist: bool = True, **untracked_payload_params: Any) -> Non
self.persist()

def persist(self) -> None:

"""Persist both payload and parameters."""

if self._payload is None:
Expand Down Expand Up @@ -274,5 +273,8 @@ def _post_load(self) -> None:

@property
def persist_filepath(self) -> Path:
params_tree_hex = md5(str({self.payload_name: self.params_tree}).encode()).hexdigest()
return self.data_dir / f"{self.payload_name}({params_tree_hex})"
return self.data_dir / f"{self.payload_name}({self.persist_hash})"

@property
def persist_hash(self) -> str:
return md5(str({self.payload_name: self.params_tree}).encode()).hexdigest()
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@

setup(
name="persistable",
version="1.2.2",
version="1.2.3",
packages=find_packages(),
url="https://github.com/aloosley/persistable",
download_url="https://github.com/aloosley/persistable/archive/1.2.2.tar.gz",
download_url="https://github.com/aloosley/persistable/archive/1.2.3.tar.gz",
license="",
author="Alex Loosley, Stephan Sahm",
author_email="[email protected], [email protected]",
Expand Down
34 changes: 30 additions & 4 deletions tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def test_tracked_persistable_dependencies(self, tmp_path: Path) -> None:
a=1, b="hello", dummy_persistable=dict(a=1, b="hello")
)

def test_persist_filepath_determined_by_both_params_tree_and_payload_name(self, tmp_path: Path) -> None:
def test_persist_hash_and_filepath_determined_by_both_params_tree_and_payload_name(self, tmp_path: Path) -> None:
# GIVEN some persistable instances
data_dir = tmp_path
params = DummyPersistableParams()
Expand All @@ -116,22 +116,27 @@ def test_persist_filepath_determined_by_both_params_tree_and_payload_name(self,
data_dir=data_dir, params=params_2, tracked_persistable_dependencies=None
)

# WHEN and THEN
initial_expected_hash = "6a0f2e637a47f02428f19726be8541a1"

# THEN
assert dummy_persistable.params_tree == params.to_dict()
assert dummy_persistable.persist_filepath == data_dir / "dummy_persistable(6a0f2e637a47f02428f19726be8541a1)"
assert dummy_persistable.persist_hash == initial_expected_hash
assert dummy_persistable.persist_filepath == data_dir / f"dummy_persistable({initial_expected_hash})"

# WHEN payload_name changed
dummy_persistable.payload_name = "another"

# THEN persist filepath has changed (hash should be determined from params_tree and payload_name)
assert dummy_persistable.params_tree == params.to_dict()
assert dummy_persistable.persist_hash == "e15cab0e04c56c6deddb1ac7bc5e5956"
assert dummy_persistable.persist_filepath == data_dir / "another(e15cab0e04c56c6deddb1ac7bc5e5956)"

# WHEN params changed
dummy_persistable.params.a += 10

# THEN persist filepath has changed (hash should be determined from params_tree and payload_name)
assert dummy_persistable.params_tree == params.to_dict()
assert dummy_persistable.persist_hash == "bd9f250dac257114768e128ed4d9eb96"
assert dummy_persistable.persist_filepath == data_dir / "another(bd9f250dac257114768e128ed4d9eb96)"

# WHEN the tracked persistable dependencies change
Expand All @@ -142,7 +147,28 @@ def test_persist_filepath_determined_by_both_params_tree_and_payload_name(self,
assert dummy_persistable.params_tree == params.to_dict() | {
dummy_persistable_2.payload_name: params_2.to_dict()
}
assert dummy_persistable.persist_hash == "e1815602bc39616f5378f6305ef2d8ee"
assert dummy_persistable.persist_filepath == data_dir / "another(e1815602bc39616f5378f6305ef2d8ee)"
assert dummy_persistable.persist_hash == "e1815602bc39616f5378f6305ef2d8ee"

# WHEN the params and persistable name are returned to their original values
dummy_persistable.payload_name = "dummy_persistable"
dummy_persistable.params = DummyPersistableParams()
dummy_persistable.tracked_persistable_dependencies = []

# THEN the hash and filepath do as well
assert dummy_persistable.persist_hash == initial_expected_hash
assert dummy_persistable.persist_filepath == data_dir / f"dummy_persistable({initial_expected_hash})"

# WHEN a new persistable is created
params = DummyPersistableParams()
new_dummy_persistable = DummyPersistable(
data_dir=data_dir, params=params, tracked_persistable_dependencies=None
)

# THEN the has is still the same as above
assert new_dummy_persistable.persist_hash == initial_expected_hash
assert new_dummy_persistable.persist_filepath == data_dir / f"dummy_persistable({initial_expected_hash})"

def test_multilevel_params_tree(self, tmp_path: Path) -> None:
# GIVEN some persistable instances that depend on each other
Expand Down Expand Up @@ -190,7 +216,7 @@ def test_payload_validation(self, tmp_path: Path) -> None:
dummy_persistable.validate_payload()
dummy_persistable.validate_payload(payload=valid_payload)
with pytest.raises(InvalidPayloadError):
dummy_persistable.validate_payload(payload=invalid_payload)
dummy_persistable.validate_payload(payload=invalid_payload) # type: ignore

def test_validate_payload_on_load(self, tmp_path: Path) -> None:
# GIVEN persistable
Expand Down

0 comments on commit 0da5c41

Please sign in to comment.