[patch] Refactor the storage interface

liamhuber commented 3 months ago

There are some public-facing methods that are removed, but honestly I'm so confident they weren't used anywhere but the plumbing that I'm still labelling this as a "patch":

I got rid of the class_name attribute and just use __class__.__name__ directly
I got rid of the global pyiron_workflow.storage.ALLOWED_BACKENDS -- this is now created dynamically as a class attribute based on what mixins are used and can be accessed from Node.allowed_backends()

And there are a couple of new interfaces:

There's some extra public facing methods around storage_directory, storage_path, etc. which are there to avoid HasStorage from inheriting from Semantic or HasWorkingDirectory but are just re-directions
delete_storage now gives an easy way to delete save files for all backends

But other than renaming for ALLOWED_BACKENDS and class_name, the tests required zero modifications, so this really is just a refactoring. (test_node has a bigger diff, but that's just because I added a cleanup failsafe that gives QoL if those tests happen to fail, which they did during development.)

This refactoring isn't perfect, for instance the fact that storage_directory, storage_path are simple re-directs to HasWorkingDirectory.working_directory and Node.graph_path, respectively (and similar redirects) is a smell that something is off in the mixin encapsulation. But I don't want to make the perfect the enemy of the good here, and this PR very nicely breaks apart the storage interfaces so that the different backends are disentangled into their own classes, which I like a lot.

Contributes to #243

github-actions[bot] commented 3 months ago

:point_left: Launch a binder notebook on branch _pyiron/pyiron_workflow/hasstorage

codacy-production[bot] commented 3 months ago

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for 52eb8139bb548fb76c87ca286b9328c12f77a1cc[^1]	:white_check_mark: 91.84%

Coverage variation details

| | Coverable lines | Covered lines | Coverage | | ------------- | ------------- | ------------- | ------------- | | Common ancestor commit (52eb8139bb548fb76c87ca286b9328c12f77a1cc) | Report Missing | Report Missing | Report Missing | | | Head commit (f93bcf1ace358d9f793c039283da0c25a74160b8) | 3390 | 2971 | 87.64% | **Coverage variation** is the difference between the coverage for the head and common ancestor commits of the pull request branch: ` - `

Diff coverage details

| | Coverable lines | Covered lines | Diff coverage | | ------------- | ------------- | ------------- | ------------- | | Pull request (#260) | 196 | 180 | **91.84%** | **Diff coverage** is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: `/ * 100%`

See your quality gate settings Change summary preferences

_{You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation} [^1]: Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

pyiron / pyiron_workflow