Ticket #11625 (closed: fixed)

Opened 5 years ago

Last modified 5 years ago

Treat pyparsing as optional dependency in PoldiCreatePeaksFromFile

Reported by: Michael Wedel Owned by: Michael Wedel
Priority: major Milestone: Release 3.4
Component: Diffraction Keywords: POLDI
Cc: Blocked By:
Blocking: Tester: Pete Peterson

Description

As the pyparsing module is not a dependency on Linux systems, the algorithm fails to load on systems where the module is not available.

Add check on registration as in DSFinterp.py.

Change History

comment:1 Changed 5 years ago by Michael Wedel

  • Status changed from new to inprogress

Refs #11625. Properly handle pyparsing dependency

Changeset: 883345f3af447b7c5793ca2e558edffd925a2286

comment:2 Changed 5 years ago by Michael Wedel

Refs #11625. Short note in documentation

Changeset: f482f80de3b4a56203f32061de5984bbab726b43

comment:3 Changed 5 years ago by Michael Wedel

Refs #11625. Encapsulating whole definition in try/catch block

Just the registration did not seem to be enough, as the parser class depends on pyparsing. Encapsulating everything into one try/catch block solves the issue.

Changeset: 64df36acebfad99074b6896e6600f15918380dc7

comment:4 Changed 5 years ago by Michael Wedel

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

This is being verified as ​pull request #642.

comment:5 Changed 5 years ago by Pete Peterson

  • Status changed from verify to verifying
  • Tester set to Pete Peterson

comment:6 Changed 5 years ago by Pete Peterson

Rather than re-indenting the whole class, just have the try/catch around the import and register. See [DSFinterp](​https://github.com/mantidproject/mantid/blob/master/Code/Mantid/Framework/PythonInterface/plugins/algorithms/DSFinterp.py#L114) as a better way to go. The class is still fully defined, just not registered if the dependency is missing.

comment:7 Changed 5 years ago by Michael Wedel

I tried that before, but then theres an error that the symbol "Word" is undefined, which is used in the parser.

comment:8 Changed 5 years ago by Michael Wedel

Refs #11625. Moving class variables

After some advice from Pete, this looks much better.

Changeset: bfc85b46004746592e24ad9af58f34856911a7c3

comment:9 Changed 5 years ago by Michael Wedel

Of course I forgot to fix the unit test - will do it when I'm on a real computer again.

comment:10 Changed 5 years ago by Michael Wedel

Refs #11625. Disable unit test when algorithm not registered

Changeset: a2d54d37b6c2c6c8992dcad5920d1ab6fbc7cfb6

comment:11 Changed 5 years ago by Pete Peterson

  • Status changed from verifying to closed

Merge pull request #642 from mantidproject/11625_optional_pyparsing_dependency

Do not register PoldiCreatePeaksFromFile when pyparsing is not installed

Full changeset: ​d185b3f0cda01ad6b8b29b64aad69604504f18ce

comment:12 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​12463

Note: See TracTickets for help on using tickets.