Create mantid dataset for published data

[Jay Rainey]

Currently, when publishing a datafile to the archives it is stored in the ​Default (first) dataset alongside the raw experiment data. Instead, analyzed data published from Mantid should be stored in a separate dataset (named mantid).

To achieve this, I need to:

1. Loop over the datasets for the specific investigation.
2. Set the ​datasetID to the dataset ID of the mantid dataset if it exists.
3. When the loop is complete, check if datasetID is still it's default value. If it is, then no mantid dataset was found. As no dataset was found, we need to create one via createMantidDataset*.

* This method will create a dataset named mantid for a given investigationID. I will need to call isAccessAllowed prior to create to verify if I can create the dataset. If I cannot then I should return -1 and output a notice().

[Jay Rainey]

Add generic isAccessAllowed method. Refs #9376.

Changeset: 459c6b01b5ca1950a2932e17ade79f4886281727

[Jay Rainey]

Use isAccessAllowed for publish GUI. Refs #9376.

Changeset: e753685a3932579bd3a1c223d561b0efa5779ae1

[Jay Rainey]

Add a method to search ICAT. Refs #9376.

Changeset: 6848ee831f915cb238f620360acc36166032c589

[Jay Rainey]

Use new search method. Refs #9376.

Changeset: 8cc230e68bff91581e405b38ee780854c0dc43cc

[Jay Rainey]

Forgot to add search to getMantidDatasetId. Refs #9376.

Changeset: 1aa471c75442a95e4786f9db4487d073dd193e55

[Jay Rainey]

Tidy return logic. Refs #9376.

Changeset: c2e738a2345dc9965b25b11c8318e4411c730ae1

[Jay Rainey]

Removed unnecessary icat4 namespace prefixes. Refs #9376.

Changeset: 21e18466616766eb95a9016b0a413905031bead1

[Jay Rainey]

Remove debug statement. Refs #9376.

Changeset: 61a88e9cc900ac82c5d9dd02690fcf311a1f7295

[Jay Rainey]

Move method to align with header order. Refs #9376.

Changeset: eb99ecc435d563d5c6921ca350c1942f410cfbee

[Jay Rainey]

Added createMantidDataset method. Refs #9376.

Changeset: 1bc871a666e800197da79c619d0af94edf544c8a

[Jay Rainey]

Create a mantid dataset if none exist. Refs #9376.

Changeset: 2a4fe517a558b7ec3f0a7c93b1d0801f58b2d352

[Jay Rainey]

Added debugging statements for publishing. Refs #9376.

Changeset: dafe1289b589b0503798adc45f2b351b78a6d63a

[Jay Rainey]

Overview

This ticket deals with separating analyzed data and experimental data in ICAT. I have opted to create a new dataset (named mantid) for the investigation the user wants to relate files to when they try to publish data. If the mantid dataset already exists, then we PUT the files there.

Testing publishing functionality

Note: You must use the mantid ICAT test account to verify this ticket as it has an investigation in my data.

1. Open the Search GUI and check My Data only. Ensure that Instrument name is empty. (This way it will return all investigations).
2. Copy the Investigation ID of Mantid Test Investigation. (It's 1193002).
3. From the Algorithms menu enter CatalogGetDataSets.
4. Enter the copied investigation ID (1193002) into the InvestigationID field, and input testingTicket into the OutputWorkspace field and click Run.
5. Right click the workspace and click Show data. Observe that there are two datasets for this specific investigation, and take note of the Number of datafiles in the mantid dataset.
6. Open the publish dialog and publish a datafile.
7. Execute CatalogGetDataSets again as above, and observe that the Number of datafiles in mantid has increased. This proves that datafiles are not published to the mantid dataset.
8. Use the Search GUI to find the datafile you just published, and load it into Mantid.

Testing searching functionality

As the createMantidDataset performs several actions (to ensure create is successful and valid) I re-factored the searching of ICAT into a separate method named performSearch. As such, all ICAT functionality that performs searches will also have to be tested, which includes:

1. Instrument name is populated correctly via CatalogListInstruments on the ICAT GUI.
2. Investigation type is populated correctly via CatalogListInvestigationTypes on the ICAT GUI.
3. Perform searches on the ICAT GUI works correctly.
4. Publishing datafiles works correctly (getMantidDatasetId is used to search for the dataset ID of the investigation to publish to).
5. CatalogMyData needs to be run via the Algorithms menu.

Perform a code review to ensure the changes made are sensible.

[Russell Taylor]

There's an Intel compiler warning here that shows why it's better to always put brackets around if-else clauses even if they're only one line:

​http://builds.mantidproject.org/job/develop_incremental/label=osx-10.8-build/110/warnings24Result/?

[Jay Rainey]

Add braces to address Intel compiler warning. Refs #9376.

Changeset: 770ab5bda8cdecbb2ae9bed32bd86a0d6b3f374c

[Peter Parker]

Changes are sensible. New functionality works as expected and the existing search functionality seems to be unaffected.

Closing.

[Peter Parker]

Merge remote-tracking branch 'origin/feature/9376_icat_create_mantid_dataset'

Full changeset: ​0455fbf70e139c22cde2c2b368c1de8fcc280d98

[Peter Parker]

Replying to Russell Taylor:

There's an Intel compiler warning here that shows why it's better to always put brackets around if-else clauses even if they're only one line:

​http://builds.mantidproject.org/job/develop_incremental/label=osx-10.8-build/110/warnings24Result/?

I'm not wholly convinced this is an argument against bracket-less condition statements (which, I'll admit, I like and use a lot). Surely the code before and after Jay's fix is logically equivalent as per the standard and so it is the compiler at fault here?

Are there other reasons why we should be avoiding them? It seems to me they're as easy / hard to read as condition statements in Python, and that always adding brackets as a rule takes more effort than only adding brackets when they're necessary. I am open to other arguments, though.

[Jay Rainey]

Replying to Peter Parker:

The Intel error above was not related to braces. I wrote a short-hand if/else and forgot to put a return statement in the else after I commented out the return method (as it did not exist on develop, e.g. something like ​this).

[Russell Taylor]

The code that triggered the warning looked like this:

905       if (icat.search(&request,&response) == SOAP_OK)
906         searchResults = response.return_;
907       else
908         // throwErrorMessage(icat);
909 
910       return searchResults;

Because whitespace doesn't matter, the return statement 'belonged' to the else clause, and the if path was then missing any return statement, triggering the warning.

If there had been braces then the commenting out would have left an empty else clause, so perhaps my comment should have also been that tickets shouldn't be closed with commented out code (though in this case I think develop had somehow diverged anyway so this code wasn't in the branch).

I tend not to like brace-less single line if-elses because of the risk that someone will add a line and it will not do what they expect. What I often do for these is put them on the same line, i.e.:

  if (test) doSomething;
  else doSomethingElse;

[Peter Parker]

Okay, thanks for the clarification guys.

[Stuart Campbell]

This ticket has been transferred to github issue ​10219