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

Consider regex match when issuing "unrecognized flow operation warning. #804

Merged
merged 6 commits into from
Feb 16, 2024

Conversation

joaander
Copy link
Member

@joaander joaander commented Feb 2, 2024

Description

Evaluate regex matches for operation/group names first, then issue a warning when there are no matches.

Motivation and Context

Previously, flow would issue warnings for all regex names as they do not exactly match the name of any group. This code demonstrates:

import signac
import flow


class Project(flow.FlowProject):
    pass


@Project.operation
def operation(job):
    pass


if __name__ == "__main__":
    project = Project()
    job = project.open_job({}).init()

    project.print_status(operation=['op.*', 'no_op', 'ab.*'], hide_progress=True)
    project.run(names=['op.*', 'no_op', 'ab.*'])

The output before the changes includes op.* which matches operation.

❯ python regex.py > /dev/null
Unrecognized flow operation(s): op.*, no_op, ab.*
Unrecognized flow operation(s): op.*, no_op, ab.*

The output after the changes includes only names that do not match.

❯ python regex.py > /dev/null
New Unrecognized flow operation(s): no_op, ab.*
New Unrecognized flow operation(s): no_op, ab.*

Checklist:

@joaander joaander requested review from a team as code owners February 2, 2024 15:55
@joaander joaander requested review from vyasr and klywang and removed request for a team February 2, 2024 15:55
Copy link

codecov bot commented Feb 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f36bb3a) 69.27% compared to head (e5e9e8b) 69.43%.
Report is 16 commits behind head on main.

❗ Current head e5e9e8b differs from pull request most recent head ab8d080. Consider uploading reports for the commit ab8d080 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #804      +/-   ##
==========================================
+ Coverage   69.27%   69.43%   +0.16%     
==========================================
  Files          48       48              
  Lines        4397     4391       -6     
  Branches     1065     1062       -3     
==========================================
+ Hits         3046     3049       +3     
+ Misses       1142     1135       -7     
+ Partials      209      207       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@cbkerr cbkerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

Copy link
Member

@b-butler b-butler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice quality of life improvement.

@b-butler b-butler merged commit f3cf4ac into main Feb 16, 2024
9 checks passed
@b-butler b-butler deleted the fix-invalid-op-warning branch February 16, 2024 15:40
@cbkerr cbkerr added this to the 0.28.0 milestone Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants