Skip to content

Comments

Solve performance issue#93

Open
cghielmini wants to merge 15 commits intomainfrom
fix_issue_performance
Open

Solve performance issue#93
cghielmini wants to merge 15 commits intomainfrom
fix_issue_performance

Conversation

@cghielmini
Copy link
Collaborator

@cghielmini cghielmini commented Feb 11, 2026

This PR addresses all improvements mentioned in issue #91 :

  • Clearer CLI/config naming (timing_regex renamed in log_file).
  • Time regex is now more robust.
  • CLI help description for timing_database is accurate and informative.

Copy link
Collaborator

@huppd huppd left a comment

Choose a reason for hiding this comment

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

Thanks, this is a greate improvement, it makes using it much clearer and makes it more robust. I commited some changes in the test to be sure that now the tool covers more date formats. I only have some minor suggestions.

rstcheck = ">=6.2.4"

# Testing tools: pytest & coverage
types-python-dateutil = "^2.9.0.20260124"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to have this moved up to the non-development requirements. Also wouldn't it be enough to add python-dateutil? I probably would also just prescribe ^2.9.0, but I guess this is a matter of taste.

TIMING_START_REGEX = r"\s+L?\s*[a-zA-Z_.]+"
TIMING_ELEMENT_REGEX = r"(?:\[?\d+[.msh]?\d*s?\]? +)"
TIMING_REGEX = TIMING_START_REGEX + " +" + TIMING_ELEMENT_REGEX + "{6,20} *(?!.)"
LOG_FILE = TIMING_START_REGEX + r"\s+(?:" + TIMING_ELEMENT_REGEX + r"){6,20} *(?!.)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is actuall a timing reges, I think here it is OK to keep the previous name.

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.

2 participants