Ticket #5658 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

Extending Instrument::getComponentByName

Reported by: Anders Markvardsen Owned by: Karl Palmen
Priority: critical Milestone: Release 2.3
Component: Mantid Keywords:
Cc: Blocked By:
Blocking: Tester: Owen Arnold

Description

Here purpose is to extend the C++ method:

Instrument::getComponentByName (const std::string & cname, int nlevels=0)

such that cname can be extended to accept a fully “qualified prefixed path” to a component name. The use case is to allow users to uniquely specify a component, where one or more such components have the same name in the IDF.

For example, imagine we have two banks: “bank 1” and “bank 2” both have a detector with name “det 1”, then the idea here is to allow a syntax, for cname, along the lines of:

bank 1/det 1

There is obviously an issue if an instrument happens to have a component called e.g. “bob/alice”. One way around this would be to have an additional argument to getComponentByName:

getComponentByName(const std::string & cname, int nlevels=0, bool fullyQualifiedPath=false)

here called fullyQualifiedPath which defaults to false, i.e. unless explicitly set to true getComponentByName keeps it original behaviour.

Change History

comment:1 Changed 8 years ago by Karl Palmen

fullyQualifiedPath=True would require a completely different (and more efficient) search algorthm to the existing fullyQualifiedPath=False.

comment:2 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.2 to Release 2.3

Moved at the end of release 2.2

comment:3 Changed 8 years ago by Karl Palmen

I think there could be a quite simple solution, which involves moving

Instrument::getComponentByName (const std::string & cname, int nlevels=0)

to superclass Component or ComponentAssembly.

Then one could for example look for tube025 in door4, where door4 is unique, but tube025 occurs several times, although unique to door4. This would save the user from having to provide a full qualified prexed path name.

comment:4 Changed 8 years ago by Karl Palmen

Nevertheless it would be helpful to make the fully qualified path name available.

This could recursively call CompAssembly::getComponentByName() with the first part taken off the fully qualified path until

(1) the fully qualified path name has only one part

or

(2) the returned Component cannot be cast to CompAssembly, and so has no children.

comment:5 Changed 8 years ago by Karl Palmen

The fullyQualifiedPath is not necessary. It can be implied by a '/' in the name.

comment:6 Changed 8 years ago by Karl Palmen

  • Status changed from new to accepted

comment:7 Changed 8 years ago by Karl Palmen

Moved getComponentByName from Instrument to CompAssembly re #5658

This will hopefully enable recursive calling on the CompAssembly got from the returned Component.

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: b1745eea93a4594fbc318f0e9f72cc160c67370c

comment:8 Changed 8 years ago by Karl Palmen

Added recursive code re #5658

New code will compile, but needs testing. Added getComponentByName to ObjCompAssembly.cpp (throws NotImplemented exception) to enable getComponentByName to occur in ICompAssembly.h.

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 4cf480db6c11045fe00469bfe11110f1c8bfe8bb

comment:9 Changed 8 years ago by Karl Palmen

Fix string splitting error re #5658

Also made it not crash in response to "rubbish/bob" and improved comments.

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 4b15e13b913566f308b82d9d064868d6d643f65c

comment:10 Changed 8 years ago by Karl Palmen

Make nlevels apply stepwise to strings with '/' re #5658

Also added comments to explain how nlevels applies in such a case.

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 4c95ad522b48cd710464abe627d6a89e4e8cadbd

comment:11 Changed 8 years ago by Karl Palmen

Extended unit test re #5658

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 8104aeaa0b268771b68d2f0a74a9c20f6c0b540f

comment:12 Changed 8 years ago by Karl Palmen

Extended unit test may need moving from InstrumentTest to CompAssemblyTest, but this can be dealt with in another ticket. The extension here is for the use cases of this ticket.

comment:13 Changed 8 years ago by Karl Palmen

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

comment:14 Changed 8 years ago by Russell Taylor

Re #5658. Clear compiler warnings.

Changeset: 923d932b074dd85f344ab8b804e24df3bff86375

comment:15 Changed 8 years ago by Karl Palmen

Moved getComponentByName from Instrument to CompAssembly re #5658

This will hopefully enable recursive calling on the CompAssembly got from the returned Component.

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: b1745eea93a4594fbc318f0e9f72cc160c67370c

comment:16 Changed 8 years ago by Karl Palmen

Added recursive code re #5658

New code will compile, but needs testing. Added getComponentByName to ObjCompAssembly.cpp (throws NotImplemented exception) to enable getComponentByName to occur in ICompAssembly.h.

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 4cf480db6c11045fe00469bfe11110f1c8bfe8bb

comment:17 Changed 8 years ago by Karl Palmen

Fix string splitting error re #5658

Also made it not crash in response to "rubbish/bob" and improved comments.

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 4b15e13b913566f308b82d9d064868d6d643f65c

comment:18 Changed 8 years ago by Karl Palmen

Make nlevels apply stepwise to strings with '/' re #5658

Also added comments to explain how nlevels applies in such a case.

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 4c95ad522b48cd710464abe627d6a89e4e8cadbd

comment:19 Changed 8 years ago by Karl Palmen

Extended unit test re #5658

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 8104aeaa0b268771b68d2f0a74a9c20f6c0b540f

comment:20 Changed 8 years ago by Owen Arnold

  • Status changed from verify to verifying
  • Tester set to Owen Arnold

comment:21 Changed 8 years ago by Owen Arnold

  • Status changed from verifying to closed

This is an internal component, but unit tests indicate that this is working.

comment:22 Changed 8 years ago by Russell Taylor

Re #5658. Clear compiler warnings.

Changeset: 923d932b074dd85f344ab8b804e24df3bff86375

comment:23 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 6504

Note: See TracTickets for help on using tickets.