Update user checks #146
Update user checks #146
Conversation
Create new global_sequencer variable in __init__ and replace references to RESOURCES["SEQUENCER"]
Add type-hinting for check classes
Typehint uses of CheckClass. Rename CheckClass _min/_max attrs to remove misleading "_" from start. Replace publishing entire check target with just a pre-formatted name: CheckClass.target_name (only part used). Condense GUI and cmd_line display formatting methods for check classes.
Rework checks.py so all formatting is done here instead of in each UI. Now pass a different CheckResult object to the sequencer - just contains necessary data for updating sequencer status and info for UI display/logging. Add new check chk_in_deviation_equal. Add tests for checks
black formatting
|
Creating request now to get review - still have a couple of things to test... |
black formatting
Re-configure check dataclasses Add type-hinting Increase test coverage for checks
Fix type hinting exception
test/core/test_checks.py
Outdated
| # (chk_log_value, [123.456], {"fmt": ".2f"}, ""), | ||
| # (chk_log_value, [123], {"fmt": ".2f"}, ""), | ||
| # (chk_equal, ["string", "string"], {}), | ||
| # (chk_equal, [1e5, 1e5], {}), | ||
| # (chk_equal, [[1,2,3,4], [1,2,3,4]], {}), | ||
| # (chk_equal, [(1,2,3,4), (1,2,3,4)], {}), | ||
| # (chk_equal, [{"a":1, "b":2}, {"a":1, "b":2}], {}), | ||
| # (chk_equal, [2+1j, 2+1j], {}), |
test/core/scripts/basichierachy.py
Outdated
| from fixate.config import RESOURCES | ||
| import fixate | ||
|
|
||
| __version__ = "1" |
src/fixate/__init__.py
Outdated
| global_sequencer = fixate.sequencer.Sequencer() | ||
| # Plan to deprecate calls to RESOURCES[] and gradual replace. | ||
| # Keep here now for backwards-compatibility with test_scripts | ||
| fixate.config.RESOURCES["SEQUENCER"] = global_sequencer |
There was a problem hiding this comment.
I've been slowly trying to remove any object instantiation from happening at import time. Depending on how tangled things are, it might be a good change to do that here? The main entry point would create the Sequencer object and assign it to the global at startup. (this is probably something that should be deferred to a follow PR)
There was a problem hiding this comment.
Note that this could be a complete mess, because test scripts at the moment might rely on reference to the Sequencer at import time :(
There was a problem hiding this comment.
Reverted and moved to a separate branch. Nearly all our current test scripts fails the new atzip.pyz checks due to this, so will spend a bit more time cleaning it up there.
Delete commented code Add type-hinting
Revert global sequencer changes and move to separate branch
|
|
||
| fixate.config.load_config(args.config) | ||
| fixate.config.load_dict_config({"log_file": args.log_file}) | ||
| # Could this be replaced with a simple log_file variable in fixate.config ^ ? |
There was a problem hiding this comment.
That's a good question, I wouldn't mind talking to @clint-lawrence about the motivation for using the load_dict_config method. But, I don't think it's something that's pertinent to this PR. @clint-lawrence, do you think it would be worth enabling discussions so that things like this can be flagged there instead of random comments in code?
There was a problem hiding this comment.
Could just create an issue?
There was a problem hiding this comment.
Yeah, I had thought that, but I had a Wall-E spork moment, and didn't really think that it's an issue per se. But happy to stick with using issues.
jcollins1983
left a comment
jcollins1983
left a comment
There was a problem hiding this comment.
@John2202W Can I get you to respond to the few comments I've left and resolve the conflicts? After that I'll be happy to merge the PR.
F-strings and remove dead code
Update user checks - release notes that belong to PR #146
4 participants
Rework checks:
New global sequencer variable:Add new variable for global sequencer (fixate.global_sequencer) to allow type-hints and improve traceability (still stashed in the RESOURCES dict for backwards compatibility in test_scripts).