python-cachier / cachier

Persistent, stale-free, local and cross-machine caching for Python functions.
MIT License
534 stars 60 forks source link

refactor: cleaning of using package/global defaults #193

Closed Borda closed 6 months ago

Borda commented 6 months ago

While Working on the package, I found the default challenging to follow... I go that package want to have one global setting but as dist it hard to trace and eventually help with refactor by any IDE, so using dataclass would be better.

Also, for user, I think using default directly with the cashier would be much better, see additional suggestion

def cachier(
    hash_func: _Type_HashFunc = _default_hash_func,
    hash_params: Optional[_Type_HashFunc] = None,
    backend: _Type_Backend = "pickle",
    mongetter: Optional[_Type_Mongetter] = None,
    stale_after: datetime.timedelta = datetime.timedelta.max,
    next_time: bool = False,
    cache_dir: Union[str, os.PathLike] = "~/.cachier/",
    pickle_reload: bool = True,
    separate_files: bool = False,
    wait_for_calc_timeout: int = 0,
    allow_none: bool = False,
):
    ...
codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (6de95e8) 97.84% compared to head (8d258d7) 97.86%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/python-cachier/cachier/pull/193/graphs/tree.svg?width=650&height=150&src=pr&token=fhsTDs7HL9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier)](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier) ```diff @@ Coverage Diff @@ ## master #193 +/- ## ========================================== + Coverage 97.84% 97.86% +0.02% ========================================== Files 6 8 +2 Lines 511 516 +5 Branches 95 88 -7 ========================================== + Hits 500 505 +5 Misses 10 10 Partials 1 1 ``` | [Files](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier) | Coverage Δ | | |---|---|---| | [cachier/\_types.py](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier#diff-Y2FjaGllci9fdHlwZXMucHk=) | `100.00% <100.00%> (ø)` | | | [cachier/config.py](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier#diff-Y2FjaGllci9jb25maWcucHk=) | `100.00% <100.00%> (ø)` | | | [cachier/core.py](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier#diff-Y2FjaGllci9jb3JlLnB5) | `100.00% <100.00%> (ø)` | | | [cachier/cores/base.py](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier#diff-Y2FjaGllci9jb3Jlcy9iYXNlLnB5) | `100.00% <100.00%> (ø)` | | | [cachier/cores/memory.py](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier#diff-Y2FjaGllci9jb3Jlcy9tZW1vcnkucHk=) | `100.00% <100.00%> (ø)` | | | [cachier/cores/mongo.py](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier#diff-Y2FjaGllci9jb3Jlcy9tb25nby5weQ==) | `95.16% <100.00%> (+0.24%)` | :arrow_up: | | [cachier/cores/pickle.py](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier#diff-Y2FjaGllci9jb3Jlcy9waWNrbGUucHk=) | `100.00% <100.00%> (ø)` | | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier). Last update [6de95e8...8d258d7](https://app.codecov.io/gh/python-cachier/cachier/pull/193?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cachier).
Borda commented 6 months ago

this requires a README change - make sure to update it (it seems to me this only touches on internal interfaces, not something user-facing, but make sure this is the case)

Correct, I made this change limited so it shall not have any impact on a user

Borda commented 6 months ago

@shaypal5 any thought why this single test is failing test_wait_for_calc_applies_dynamically[pickle-None]? also only on Unix machine, Wil is fine or skipped...

shaypal5 commented 6 months ago

No idea. I suggest using git blame to track down the PR, contributor and issue context; this probably tests a very specific issue, and finding the discussion and the code contributed alongside the test should help.

Good luck! :)

(if it's me let me know and I'll delve into my memory vault)

Borda commented 6 months ago

@shaypal5 mind checking it again? :flamingo: