Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose conditions #44

Merged
merged 53 commits into from
Jul 11, 2022
Merged

Expose conditions #44

merged 53 commits into from
Jul 11, 2022

Conversation

Miksus
Copy link
Owner

@Miksus Miksus commented Jul 10, 2022

This is quite a massive update. Nothing already documented will change but quite a lot under the hood.

Most notable changes:

Introduction of easy to use conditions

These are convenient alternatives for those who prefer Python objects instead of the string syntax. These are exact equivalents:

from rocketry.conds import daily, time_of_week

@app.task(daily)
def do_first():
    ...

@app.task(daily.between("12:00", "15:00") & time_of_week.on("Monday"))
def do_second():
    ...

and

from rocketry.conds import daily, time_of_week

@app.task("daily")
def do_first():
    ...

@app.task("daily between 12:00 and 15:00 & time of week on Monday")
def do_second():
    ...

Pick which style suits you.

Parametrizable conditions

Previously the conditions used sessions and tasks in quite an ugly way. They relied on that they are mutable and

Now instead of using bool(condition), this should be used instead: condition.observe(session=session). The context is given when evaluating instead of embedding the context inside.

In addition, this allows easier custom conditions and conditions support for passing the context out of the box similarly as parametrization in tasks work. Actually, they now share the mechanism.

Easier reference

Now these are also possible:

from rocketry.conds import daily, time_of_week

@app.cond()
def is_foo():
    ...

@app.task(daily & is_foo)
def do_things():
    ...

app.cond() now returns the condition itself. There could be problems with pickling but conditions are not passed to processes.

Also, reference in Return is also simpler:

from rocketry.args import Return

@app.task()
def do_first():
    ...

@app.task()
def do_second(arg=Return(do_first)):
    ...

Note how we did not pass the task name as a string but just the function. The task name is stored in the function as do_first.__rocketry__['name'].

Miksus added 30 commits July 7, 2022 08:57
That is unnecessary and no longer should be used.
Failed as True has no attribute observe
This subpackage contains medium layer
API to the conditions. Easier than raw condition classes.
Comparisons were as strings instead of ints when created using string
syntax.
@Miksus Miksus added enhancement New feature or request core Relates to the core functionalities (ie. base classes) built-in Relates to the built-in tasks, conditions etc. labels Jul 10, 2022
For some reason TaskCond broke. It's due to that pydantic respects the
classvar order and for some reason that got affected even though the
attributes were not changed. Maybe it is relying on very fine state
that is slightly random.
@codecov-commenter
Copy link

codecov-commenter commented Jul 10, 2022

Codecov Report

Merging #44 (b61468a) into master (766fd57) will decrease coverage by 0.25%.
The diff coverage is 98.23%.

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   90.08%   89.83%   -0.26%     
==========================================
  Files          81       83       +2     
  Lines        3652     3747      +95     
==========================================
+ Hits         3290     3366      +76     
- Misses        362      381      +19     
Impacted Files Coverage Δ
rocketry/core/condition/__init__.py 100.00% <ø> (ø)
rocketry/conditions/task/task.py 92.12% <94.82%> (-3.45%) ⬇️
rocketry/conditions/task/utils.py 90.47% <96.42%> (+1.28%) ⬆️
rocketry/conditions/api.py 97.22% <97.22%> (ø)
rocketry/core/condition/base.py 92.26% <98.73%> (+1.94%) ⬆️
rocketry/_setup.py 100.00% <100.00%> (ø)
rocketry/args/builtin.py 94.91% <100.00%> (+0.27%) ⬆️
rocketry/conditions/__init__.py 100.00% <100.00%> (ø)
rocketry/conditions/func.py 87.80% <100.00%> (+1.69%) ⬆️
rocketry/conditions/meta.py 95.74% <100.00%> (+0.50%) ⬆️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 766fd57...b61468a. Read the comment docs.

@Miksus
Copy link
Owner Author

Miksus commented Jul 11, 2022

If session.task_exists is tested the code coverage goal is probably reached. The cov decreased due to removal of unneeded code (which was tested).

@Miksus Miksus merged commit d43c9d3 into master Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
built-in Relates to the built-in tasks, conditions etc. core Relates to the core functionalities (ie. base classes) enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: Expose the condition classes as substitute for the string based conditions
2 participants