Ticket #9376 (closed: fixed)
Create mantid dataset for published data
Reported by: | Jay Rainey | Owned by: | Jay Rainey |
---|---|---|---|
Priority: | major | Milestone: | Release 3.2 |
Component: | Framework | Keywords: | ICAT |
Cc: | Blocked By: | ||
Blocking: | Tester: | Peter Parker |
Description (last modified by Jay Rainey) (diff)
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:
- Loop over the datasets for the specific investigation.
- Set the datasetID to the dataset ID of the mantid dataset if it exists.
- 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().
Change History
comment:5 Changed 6 years ago by Jay Rainey
- Status changed from assigned to inprogress
Add generic isAccessAllowed method. Refs #9376.
Changeset: 459c6b01b5ca1950a2932e17ade79f4886281727
comment:6 Changed 6 years ago by Jay Rainey
Use isAccessAllowed for publish GUI. Refs #9376.
Changeset: e753685a3932579bd3a1c223d561b0efa5779ae1
comment:7 Changed 6 years ago by Jay Rainey
Add a method to search ICAT. Refs #9376.
Changeset: 6848ee831f915cb238f620360acc36166032c589
comment:8 Changed 6 years ago by Jay Rainey
Use new search method. Refs #9376.
Changeset: 8cc230e68bff91581e405b38ee780854c0dc43cc
comment:9 Changed 6 years ago by Jay Rainey
Forgot to add search to getMantidDatasetId. Refs #9376.
Changeset: 1aa471c75442a95e4786f9db4487d073dd193e55
comment:10 Changed 6 years ago by Jay Rainey
Tidy return logic. Refs #9376.
Changeset: c2e738a2345dc9965b25b11c8318e4411c730ae1
comment:11 Changed 6 years ago by Jay Rainey
Removed unnecessary icat4 namespace prefixes. Refs #9376.
Changeset: 21e18466616766eb95a9016b0a413905031bead1
comment:12 Changed 6 years ago by Jay Rainey
Remove debug statement. Refs #9376.
Changeset: 61a88e9cc900ac82c5d9dd02690fcf311a1f7295
comment:13 Changed 6 years ago by Jay Rainey
Move method to align with header order. Refs #9376.
Changeset: eb99ecc435d563d5c6921ca350c1942f410cfbee
comment:14 Changed 6 years ago by Jay Rainey
Added createMantidDataset method. Refs #9376.
Changeset: 1bc871a666e800197da79c619d0af94edf544c8a
comment:15 Changed 6 years ago by Jay Rainey
Create a mantid dataset if none exist. Refs #9376.
Changeset: 2a4fe517a558b7ec3f0a7c93b1d0801f58b2d352
comment:17 Changed 6 years ago by Jay Rainey
- Status changed from inprogress to verify
- Resolution set to fixed
Added debugging statements for publishing. Refs #9376.
Changeset: dafe1289b589b0503798adc45f2b351b78a6d63a
comment:18 Changed 6 years ago by 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.
- Open the Search GUI and check My Data only. Ensure that Instrument name is empty. (This way it will return all investigations).
- Copy the Investigation ID of Mantid Test Investigation. (It's 1193002).
- From the Algorithms menu enter CatalogGetDataSets.
- Enter the copied investigation ID (1193002) into the InvestigationID field, and input testingTicket into the OutputWorkspace field and click Run.
- 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.
- Open the publish dialog and publish a datafile.
- 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.
- 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:
- Instrument name is populated correctly via CatalogListInstruments on the ICAT GUI.
- Investigation type is populated correctly via CatalogListInvestigationTypes on the ICAT GUI.
- Perform searches on the ICAT GUI works correctly.
- Publishing datafiles works correctly (getMantidDatasetId is used to search for the dataset ID of the investigation to publish to).
- CatalogMyData needs to be run via the Algorithms menu.
Perform a code review to ensure the changes made are sensible.
comment:19 follow-up: ↓ 26 Changed 6 years ago by 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/?
comment:20 Changed 6 years ago by Russell Taylor
- Status changed from verify to reopened
- Resolution fixed deleted
comment:21 Changed 6 years ago by Jay Rainey
- Status changed from reopened to inprogress
Add braces to address Intel compiler warning. Refs #9376.
Changeset: 770ab5bda8cdecbb2ae9bed32bd86a0d6b3f374c
comment:22 Changed 6 years ago by Jay Rainey
- Status changed from inprogress to verify
- Resolution set to fixed
comment:23 Changed 6 years ago by Peter Parker
- Status changed from verify to verifying
- Tester set to Peter Parker
comment:24 Changed 6 years ago by Peter Parker
Changes are sensible. New functionality works as expected and the existing search functionality seems to be unaffected.
Closing.
comment:25 Changed 6 years ago by Peter Parker
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/9376_icat_create_mantid_dataset'
Full changeset: 0455fbf70e139c22cde2c2b368c1de8fcc280d98
comment:26 in reply to: ↑ 19 ; follow-up: ↓ 27 Changed 6 years ago by 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.
comment:27 in reply to: ↑ 26 Changed 6 years ago by 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).
comment:28 Changed 6 years ago by 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;
comment:29 Changed 6 years ago by Peter Parker
Okay, thanks for the clarification guys.
comment:30 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 10219