draft: sketched out SO interface CI workflow. #798
Conversation
github-actions
bot
added
the
core
|
I just sketched out how this could work, but this could be a project for Andy. |
tests/ci/check_so_interface.py
Outdated
| } | ||
| freeze = {} | ||
|
|
||
| with open('tests/ci/frozen_functions.py', 'r') as fh: #opens file "fname", in read mode 'r', to be used in code as "fh" |
There was a problem hiding this comment.
what is going to be stored in this file? Is it a pseudo-python representation of the objects/methods structure that define the interface?
tests/ci/check_so_interface.py
Outdated
| return True | ||
|
|
||
| if __name__ == "__main__": #if this is the main thing being run | ||
| for fname, spec in interface.items(): #loop over file names "fname" as keys and "spec" as values in dictionary called "interface" |
There was a problem hiding this comment.
currently interface is not defined
tests/ci/check_so_interface.py
Outdated
| for meth in spec[key]: #iterating over methods specified in spec[key] | ||
| assert meth in tree[key] | ||
| assert _compare_args(spec[key][meth], tree[key][meth]) | ||
| notfound = [] |
There was a problem hiding this comment.
these two lines don't need to be in the context manager
tests/ci/check_so_interface.py
Outdated
| notfound = [] | ||
| found = {} | ||
| dump = ast.dump(tree) | ||
| for node in ast.walk(tree): |
There was a problem hiding this comment.
the first layer in the tree is usually going to be made up of ClassDef objects (though not always), which in turn contain FunctionDef's. The original sketch had a check for this.
tests/ci/check_so_interface.py
Outdated
| @@ -104,49 +130,50 @@ | |||
| for node in ast.walk(frozen): | |||
| if isinstance(node, ast.FunctionDef) and node.name in spec_args.keys(): | |||
| freeze.update({node.name: node}) | |||
| print("FROZEN") | |||
There was a problem hiding this comment.
There's a lot of these print checks I put along the way to debug, and visualize the flow of my code. I haven't deleted them yet because I wanted to keep them in here to remember good places/dictionaries and lists to check in my code for future debugging. A final version of this code will definitely have these removed.
tests/ci/check_so_interface.py
Outdated
| interface = { | ||
| "python/pysmurf/client/tune/smurf_tune.py": { # file | ||
| "SmurfTuneMixin": { # class | ||
| "methods": [ |
There was a problem hiding this comment.
what is this methods item used for? We probably only want to check for the existence of the methods that are listed in the interface document, and then this list is redundant with the individual method specs.
tests/ci/check_so_interface.py
Outdated
| for classname, funcname in classname.items(): | ||
| for funcname in funcname.keys(): | ||
| with open(fname, 'r') as fh: #opens file "fname", in read mode 'r', to be used in code as "fh" | ||
| tree = ast.parse(fh.read()) #parsing contents of file into abstract syntax tree |
There was a problem hiding this comment.
might make sense to open the file and parse the tree in the outer loop to avoid redoing it on every iteration needlessly.
tests/ci/check_so_interface.py
Outdated
|
|
||
| if __name__ == "__main__": #if this is the main thing being run | ||
| for fname, classname in interface.items(): #loop over file names "fname" as keys and "spec" as values in dictionary called "interface" | ||
| for classname, funcname in classname.items(): |
There was a problem hiding this comment.
in general I would suggest not reusing the same variable name like this, just for ease of understanding.
| spec_defaults = interface[fname][classname][funcname]["defaults"] | ||
|
|
||
| # Extract function arguments from the AST node | ||
| found_args = [arg.arg for arg in node.args.args] |
There was a problem hiding this comment.
This looks good, but you may want to think about some more complicated cases that might occur. See here for an example of all the different types of arguments that can occur. We probably don't need to worry about positional-only or kw-only args because that is a pretty unusual pattern, but at least checking for vararg and kwarg would be good.
tests/ci/check_so_interface.py
Outdated
| .dat noise files - generated by take_stream_data | ||
| iv_files - generated by run_iv" | ||
| """ | ||
| def compare_args(node, fname, classname, funcname, type): |
There was a problem hiding this comment.
As a general practice comment, this function would be more portable if it didn't rely on variables that are outside of it. Currently you pass it keys that it will use to address the interface dictionary, which is defined outside the function. Instead you could pass it only what it needs to do the comparison -- something like the contents of interface[fname][classname][funcname] and the node. Also you can probably figure out if you're dealing with a function or a class from the node object rather than pass that info as an argument. In fact, if the function is following completely different paths for functions and classes, it could also be two functions.
tests/ci/check_so_interface.py
Outdated
| found_meths.append(child.name) | ||
|
|
||
| # Compare methods to specified methods in interface dict | ||
| if found_meths != spec_meths: |
There was a problem hiding this comment.
In this case we don't care about the order of methods. Consider using the set built-in and check that the intersection of the two is complete.
Description
Draft of github CI workflow to enforce compliance with the SO interface document here.
This is just a sketch and the code needs to be fleshed out, tested and documented.