Ticket #8419 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Argument value checking for plotSpectrum, plotBin and plotMD

Reported by: Arturs Bekasovs Owned by: Federico M Pouzols
Priority: major Milestone: Release 3.3
Component: Python Keywords: NewStarter
Cc: nick.draper@… Blocked By:
Blocking: Tester: Anders Markvardsen

Description (last modified by Arturs Bekasovs) (diff)

Specifying non-existent index (spectra/bin/dimension) values:

  • plotSpectrum throws an uncaught exception
  • plotBin silently draws something, even if specified bin is -1000
  • plotMD prints warning to the log if bin index > max index, but still draws some garbage. No warning when dimension index < 0 though.

plotBin neither plots nor throws an error when non-existent workspace is specified.

From my point of view they should all throw Python-level exceptions in these cases, like plotSpectrum and plotMD do for non-existent workspaces.

Change History

comment:1 Changed 7 years ago by Arturs Bekasovs

  • Description modified (diff)

comment:2 Changed 7 years ago by Nick Draper

  • Status changed from new to assigned

bulk move to assigned at the into of the triage step

comment:3 Changed 6 years ago by Nick Draper

  • Keywords NewStarter added

comment:4 Changed 6 years ago by Nick Draper

  • Owner changed from Anyone to Federico M Pouzols
  • Milestone changed from Backlog to Release 3.3

comment:5 Changed 6 years ago by Federico Montesino Pouzols

  • Status changed from assigned to inprogress

Input error checks added: plotSpectrum, plotMD, plotBin, re #8419

Changeset: 1fbe7c455002e91e04ef51cd4309c6f965d90e14

comment:6 Changed 6 years ago by Federico Montesino Pouzols

Avoid MSVC warning about comment tags, re #8419

Changeset: b1c1b8667b998f2cc70b4158f395bc89af57b01d

comment:7 Changed 6 years ago by Federico Montesino Pouzols

Better error messages, re #8419

Changeset: d7ef24bfacc5baea77bdd9286f1893dab7821573

comment:8 Changed 6 years ago by Federico Montesino Pouzols

New mantidplotpy test: MantidPlotInputArgsCheck, re #8419

Changeset: 5ebdf5add7d8bfc38a0484aa34d7c7772b69520d

comment:9 Changed 6 years ago by Federico Montesino Pouzols

remove reminder comment, re #8419

Changeset: 19307b35a2aa60c36c10839e70c7acfd53cf1a5b

comment:10 Changed 6 years ago by Federico Montesino Pouzols

Merge branch 'bugfix/8419_better_input_check_for_python_plots' into develop, re #8419

Conflicts:

Code/Mantid/MantidPlot/CMakeLists.txt

There was no apparent true conflict

Changeset: a146322b031250cb088a6ac674f569bff42da280

comment:11 Changed 6 years ago by Federico Montesino Pouzols

Catch (good) exceptions so that jenkin's tests don't fail, re #8419

Changeset: 3173eb7f0d63f930f626e325f0db444b6248c266

comment:12 Changed 6 years ago by Federico M Pouzols

  • Status changed from inprogress to verify
  • Resolution set to fixed

I think these types of errors are properly handled now, as proposed in the ticket description.

From the script interpreter you can test with commands like these (say you have two workspaces: "MAR11060", and "TSC10076").

  • Example python commands that work:

plotSpectrum("TSC10076", 0)

plotSpectrum("MAR11060", 500)

plotBin("MAR11060", 0)

plotBin("TSC10076", 3000)

plotBin(("TSC10076", "MAR11060"), 100)

plotBin(("TSC10076", "MAR11060"), 0)

  • Example python commands that don't work (exception raised):

plotSpectrum("TSC10076", -7)

plotSpectrum("TSC10076", 149)

plotSpectrum("TSC10076", 500)

plotSpectrum(("MAR11060", "TSC10076"), 500)

plotSpectrum(("MAR11060", "TSC10076"), (3,500))

plotBin("TSC10076", -3)

plotBin("MAR11060", 2684)

plotBin(("TSC10076", "MAR11060"), 3000)

plotMD("MAR11060", 33)

All these should raise 'ValueError' with an informative error msg.

I also added a simple mantidplotpy test (Code/Mantid/MantidPlot/test/MantidPlotInputArgsCheck.py).

Last edited 6 years ago by Federico M Pouzols (previous) (diff)

comment:13 Changed 6 years ago by Federico Montesino Pouzols

with_invalid_raises_RuntimeError -> raises_ValueError, re #8419

Changeset: 4c0f9c9cfcb2b3cb3308fa1acbf90e10f31b22ba

comment:14 Changed 6 years ago by Anders Markvardsen

  • Status changed from verify to verifying
  • Tester set to Anders Markvardsen

comment:15 Changed 6 years ago by Anders Markvardsen

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/8419_better_input_check_for_python_plots'

Full changeset: ​0e1ec6d40a1211d552e96dcb3ff6ac0e403b4c0d

comment:16 Changed 6 years ago by Anders Markvardsen

Tested with sns and isis data. New unit test is passing. Good code documentation provided

comment:17 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​9263

Note: See TracTickets for help on using tickets.