Excel_Engine: `Query.IsValidColumn` and `Query.IsValidRow` to support double as input

[alelom]

Description:

In Query.IsValidColumn and Query.IsValidRow we need to allow for doubles as input, because Grasshopper sliders give double even for integer numbers.

See https://github.com/BHoM/Excel_Toolkit/pull/320/files#r762106787:

We must also support any double input that can be Parsed as int.

We can correct this by doing the int.TryParse() before the rest of the code.

Test file(s):

See above script snippet. Just drop a Create.CellAddress() component in the canvas and feed it any integer number via a slider. The Create.CellAddress() method calls Query.IsValidColumn() and Query.IsValidRow() in its first line of code.

[FraserGreenroyd]

I've just seen this issue, I'm not convinced that this is an issue with Excel_Engine, but rather with a specific UI and how it handles conversion of data? Granted I'm not fully familiar with the engine method referenced, but given how Excel works, I would imagine IsValidColumn should take in a string for letters (or an int and convert the int to a letter column heading, and IsValidRow should take in an int (based on Excel rows being numbered).

I don't think it should be the work of Excel_Engine to handle taking in a double - this is either a UI/UX problem (to be handled by the specific UI) or a user problem (to be handled by the user with clear/informative error messages). For me the key bit is:

we need to allow for doubles as input, because Grasshopper sliders give double even for integer numbers.

This highlights it's a Grasshopper problem to me, not an Excel_Engine one. I think it might be best to park this for the new year and have a wider discussion on how we handle such inputs more broadly within the UI/UX settings than try to rush a solution for this beta?

[alelom]

Partly true, but we want to allow freedom of inputs from any environment, as long as the input is interpretable as an Excel column/row input. Agreed it's also due on how the Engine method is interpreted from the UI, but tackling it from the UI perspective is much more complex.

[FraserGreenroyd]

It is, but is the much more robust/correct solution in my mind. It might be worth removing the methods entirely for this beta because versioning that after 5.0 will also become a bit of a headache, then we can have the discussion on the robustness of what's needed for different use cases and the best way forward?

[alelom]

Honestly I don't think so. I'd rather push a quick fix and get along with our lives. The gained flexibility here is just worth the method existing. I can't see what versioning has something to do here.

[alelom]

Having these methods allows to have 1 entry point for any data type indicating a column/row index. Sure we can live with type strong inputs but it means multiplying the engine methods and confusing users, or we can do as you say and solve the infinitely more complex issue from the UI.

Consider also that this is not only to do about sliders outputting double even with integer values. This is also to allow flexibility of inputting through the same Panel either a number or a text indicating a column. Currently, if your input is first passed through a slider but contains a number, you would need different IsValid method for different type input. Having one entry point only taking object simplifies enormously the life of users, and reduces the chance of being pinged for useless technical support ("my input used to work")

[alelom]

Closed for the reasons above.

[alelom]

Replaced with https://github.com/BHoM/Excel_Toolkit/issues/323 that is more generic.