MT-18 improve target string validation

[wbode-za]

validate_target adds additional target string validation

• Test for unicode
• Test for invalid separator ','
• Test against Katpoint.Target

JIRA: MT-18

[bmerry]

I don't particularly like having this in a separate function that's doing its own parsing. It's going to duplicate a lot of the functionality in the Target constructor, any new features will have to be handled in both places, and it'll be very difficult to tell whether a string might pass the constructor yet fail validation or vice versa.

I think a better design would be to incorporate the diagnosis into the Target constructor, so that the exception you get out if it fails has the sort of useful, user-friendly information that is currently being printed. Then there is no need for a separate validation function here; calling code can just attempt to construct a target and report the exception if it fails.

[ludwigschwardt]

I agree with @bmerry's observations. A library like katpoint should never have a print statement, and as little logging as possible. The real communication with the outside world should happen via the API and for errors that usually implies exceptions as the main vector.

On the other hand, if this was a validate_targets.py script living in the scripts directory I would have fewer of these objections. But then it also runs the real risk of getting out of sync.

[ajoubertza]

Thanks for the comments @bmerry and @ludwigschwardt. I agree that this is a bit unusual, and can be better. The need for it comes from the astronomers who would like a more user-friendly report of problems than a stacktrace. They are using katpoint, so that is why we want something like this in katpoint. Typically they are using Jupyter notebooks. We could run a script in the notebook. I see the %run -i 'my_script.py' syntax would let the script use the notebook's namespace, if that was necessary.

A compromise solution:

1. Keep the "pretty print validation" (pp_validation) checks in target.py, but don't print anything - just return the lines for printing.
2. In the pp_validation, first try and instantiate the Target object, i.e. validate using the constructor, which uses the canonical construct_target_params function. Only if that raises an exception, then do further parsing to try and find the source of the error.
3. The construct_target_params function reports all problems as ValueError. We could make some new exception classes inheriting from ValueError and use those instead to provide a little more detail, and guide the pp_validation - e.g. was it a unicode problem or missing coordinates?
4. Add scripts/validate_targets.py which opens the file, iterates through the targets and calls the library version of pp_validate. The script calls print.

[ajoubertza]

After discussion with @ludwigschwardt, we'll be moving this into a script, and add some custom exceptions to construct_target_params. As that will be significantly different to what we have here we are closing this PR. Thanks for your comments @bmerry - they will be applied to the new branch.