@dates_active decorator

[lars-reimann]

What problem do you want to solve?

Reference the issue or discussion, if there is any. Provide a description and/or bullet
points to describe the changes in this PR.

Closes partially #334

Todo

• Pick an appropriate title.
• Put Closes #XXXX in the first PR comment to auto-close the relevant issue once
  the PR is accepted. This is not applicable if there is no corresponding issue.
• Document PR in CHANGES.md.

[codecov]

Codecov Report

Patch coverage: 98.38% and project coverage change: +0.16 🎉

Comparison is base (02fd27c) 94.31% compared to head (cfe60bb) 94.47%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #495      +/-   ##
==========================================
+ Coverage   94.31%   94.47%   +0.16%     
==========================================
  Files          47       47              
  Lines        2850     2897      +47     
==========================================
+ Hits         2688     2737      +49     
+ Misses        162      160       -2     

Impacted Files	Coverage Δ
src/_gettsim/taxes/zu_verst_eink/eink.py	91.17% <83.33%> (+0.85%)	⬆️
src/_gettsim/functions_loader.py	86.69% <100.00%> (+0.21%)	⬆️
src/_gettsim/policy_environment.py	100.00% <100.00%> (ø)
src/_gettsim/shared.py	98.83% <100.00%> (+0.96%)	⬆️
src/_gettsim/taxes/zu_verst_eink/freibetraege.py	87.50% <100.00%> (+0.83%)	⬆️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[lars-reimann]

I only partially replaced the conditionals in load_functions_for_date with the @dates_active decorator so far to keep the PR small and allow it to be merged quickly. Other functions can be gradually migrated to the new system.

[hmgaudecker]

Wow, that looks great! Just a couple of quick comments / thoughts:

• I think it is a good idea to revert 108ab4f -- the version we will release with this should only support recent Python (easy to get with conda and gives new Python users much improved error messages), so given that Python can't distinguish between the two formats, the tests should make clear that we support both.
• It would be great to build in an automatic check that time periods do not overlap for functions that end up having the same key in the DAG. Ideally with a nice error message and early (so you notice immediately when coding things up). If that proves to be hard, fail when overwriting a key. But that should not silently work, I think (I did not check current behavior)
• @janosg commented on GEP 6 that @tobiasraabe has been using a nicer way to store function attributes than a bunch of dunder-attributes. I briefly checked the other day but only found class attributes via the attrs library, not sure whether they also work for functions?

[janosg]

My point was just that instead of several dunder attributes there should be just one (a dict containing all information). It is very easy to lose dunder attributes when applying other decorators. I don't think functools.wraps copies everything. If we have just one, each decorator implemented in gettsim can make sure it does not lose that information.

[lars-reimann]

I think it is a good idea to revert 108ab4f -- the version we will release with this should only support recent Python (easy to get with conda and gives new Python users much improved error messages), so given that Python can't distinguish between the two formats, the tests should make clear that we support both.

The behavior of datetime.date.fromisoformat was changed in Python 3.11. Previously, the dash separators were necessary, even though the ISO 8601 format doesn't require them. We could now either

1. Make our own ISO-compliant parser or use a 3rd party library to support Python 3.9 and 3.10.
2. Only support the format YYYY-MM-DD, validate the string input, and highlight this in the documentation.

In my opinion, the second option makes more sense. The dashes improve readability and we ensure consistency in our source code.

[ChristianZimpelmann]

In my opinion, the second option makes more sense. The dashes improve readability and we ensure consistency in our source code.

Sounds like the better solution to me, as well.

[ChristianZimpelmann]

Looks great, thank you!

• load_internal_functions is a nice addition!
• I like __info__ more than __gettsim__ (sorry for not responding before).

[lars-reimann]

Not sure why readthedocs is failing now but since other PRs are also affected, it seems to be unrelated to the changes in this PR.

[janosg]

It's the latest update of pydata-sphinx-theme.

We pinned it in estimagic for now. See here

[hmgaudecker]

Basically done! Thanks a lot, great work!

[hmgaudecker]

Excellent, thank you!

Let me get my hands dirty a little on porting further changes, maybe I'll have a comment or two based on that. No PRs on the horizon that will use this machinery AFAIC, else we can merge earlier.

NB: No need to change this, but I did not mean to push you towards including a docstring in the function, the comment just was not clear to me without knowing the docstring of the Exception. 😄

[lars-reimann]

NB: No need to change this, but I did not mean to push you towards including a docstring in the function, the comment just was not clear to me without knowing the docstring of the Exception. 😄

No worries, good to have the docstring anyway.

[hmgaudecker]

Truly excellent work, it has been a joy to apply it !