ISISComputingGroup / IBEX

Top level repository for IBEX stories
5 stars 2 forks source link

scripting: trap errors with non-integer range in script #8415

Open FreddieAkeroyd opened 3 months ago

FreddieAkeroyd commented 3 months ago

As a user i would like the genie python load_script linter to spot use of a non-integer in a python range command e.g.

def myfunc():
    for i in range(1, 3.5):
        print(i)

currently the script loads without error, but throws an exception at runtime

acceptance criteria

notes

See propose solution from Tom in comment bellow

if it is not possible to spot this, maybe we can define g.range() that maps to numpy.arange() which does allows floating point values

Time in planning meeting (18/07/24)

1:48:40

Tom-Willemsen commented 3 months ago

pyright with a very basic config would catch this, and other similar problems, with the following output:

c:\merlin_scripts\<snip>.py
  c:\merlin_scripts\<snip>.py:11:22 - error: Argument of type "float" cannot be assigned to parameter "start" of type "SupportsIndex" in function "__new__"
    "float" is incompatible with protocol "SupportsIndex"
      "__index__" is not present (reportArgumentType)
  c:\merlin_scripts<snip>.py:11:29 - error: Argument of type "float" cannot be assigned to parameter "stop" of type "SupportsIndex" in function "__new__"
    "float" is incompatible with protocol "SupportsIndex"
      "__index__" is not present (reportArgumentType)
  c:\merlin_scripts\<snip>.py:11:34 - error: Argument of type "float" cannot be assigned to parameter "step" of type "SupportsIndex" in function "__new__"
    "float" is incompatible with protocol "SupportsIndex"
      "__index__" is not present (reportArgumentType)

I initially thought that adding type checking to user scripts would report too many errors, but having actually tried it on some user script directories from a few different beamlines there's actually not very many errors - and those that are reported seem like genuine problems (once I filtered out files containing syntax errors, e.g. old python 2 user scripts).

So I think the best approach to this ticket would be to run pyright as part of "script checking" on user scripts on load_script, in a similar way to what we already do with pylint.

jackbdoughty commented 2 months ago

Link to genie-python PR Link to release notes PR Link to system tests PR Pyright user docs first draft

jackbdoughty commented 1 month ago

Hopefully ruff & pyright problems are all fixed and ready for review @LowriJenkins