Ticket #8962 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

SaveTbl and LoadTbl Algorithms

Reported by: Keith Brown Owned by: Keith Brown
Priority: critical Milestone: Release 3.2
Component: Reflectometry Keywords:
Cc: Blocked By:
Blocking: #8692 Tester: Owen Arnold

Description

Tbl is a file format the ISIS reflectometry gui uses for native saves and loads, but they're currently written in python.

Write a c++ algorithm for it which uses tableworkspaces

Change History

comment:1 Changed 7 years ago by Keith Brown

  • Blocking 8963 added

comment:2 Changed 7 years ago by Nick Draper

  • Status changed from new to assigned

Bulk move of tickets out of triage (new) to assigned at the introduction of the triage state

comment:3 Changed 7 years ago by Keith Brown

  • Priority changed from major to critical
  • Status changed from assigned to inprogress
  • Blocking 8692 added; 8963 removed

This just got a bump in priority as it needs to be done before we can build the new GUI. This algorithm will no longer be hooked into the old gui as a result.

The loader will split the 17 length row (5-per run,5,5,2-for entire row) into three rows of 8 (5,2,1-for stitch ID) and put them into a table workspace

The saver will need to do the reverse, seeking out columns with identical stitch IDs, and putting them into the 17 length format. However it should reject it and not save if there is a stitch group of 4 or more as this isn't backwards compatible.

Last edited 7 years ago by Keith Brown (previous) (diff)

comment:4 Changed 7 years ago by Keith Brown

Refs #8962 Skeleton classes made for save, load and tests

Classes for the new save and load algorithms, which I've called SaveReflTBL and LoadReflTBL, have been made with enough data that they'll compile but do not much else.

Test skeletons ahve also been made

Changeset: 3739439f19c915b769cb84e2b0cf0032c0efc27f

comment:5 Changed 7 years ago by Keith Brown

Refs #8962 Lines are read and converted properly.

getCells() takes a string and splits it into 17 columns (no more, no less or it'll throw) delimited by commas.

Cases for EOL characters have been included as the fileDescriptor provided in Confidence does different things than a ifstream opened manually, and i found it gave me a \r character i needed to deal with.

Changeset: 75c28712b9c29324ecfd83ede1b02f91475ad192

comment:6 Changed 7 years ago by Keith Brown

refs #8962 Move extractToEOL from LogParser to Strings

The extractToEOL Kernal namespace method has been moved to the Kernal::Strings namespace where it makes more sense to be alongside other string and ascii stream methods

The change was made because of the difficulties i was having with the binary files stream provided by the FileDescriptor.

Changeset: 1fe415324794bc627314e8e09eb59962da13131b

comment:7 Changed 7 years ago by Keith Brown

Refs #8962 extractToEOL now used instead of getline

Changed calls to std::getline to Mantid::Kernel::Strings::extractToEOL in order to make sure I don't read in any EOL characters

Changeset: a820bda8623ca0e5f003a855e01b552aa1109cf0

comment:8 Changed 7 years ago by Keith Brown

Refs #8962 Added ITableWorkspace to Load's output workspace types

ITableWorkspace has been added to the list of output workspace types load is expecting to output.

Changeset: fcd1fd68a2871e6bfc156295cafc8d6c21be6966

comment:9 Changed 7 years ago by Keith Brown

Refs #8962 LoadReflTBL complete and refactored

getCells has been refactored into a nubmer of smaller methods and now returns size_t.

The algorithm now builds and outputs a tableworkspace like it should, and ignores blank lines.

Changeset: 400c8bffbddf0c2e0366ed33c8cab7b51621a726

comment:10 Changed 7 years ago by Keith Brown

Refs #8962 Tweaked column headers, Added Docs

Added some comments to the content checks to explian them more.

Tweaked the columns headers to have no plot type and altered a couple of names

Changeset: 68aa4ce205a9eb4fec760a56f2c64c4544b861ec

comment:11 Changed 7 years ago by Keith Brown

Refs #8962 Fixes to loader

The loader had a problem where large groups of commas at the start of the row would cause the loader to become confused and skip a cell. Fixed so that it now looks if it was the first column or not, rather than looking at the place of the last found comma.

The stitch ID has been fixed to be an int to help the save algorithm

Changeset: 215c628177b38b05db3e6eddb779aacbd0dd1f3c

comment:12 Changed 7 years ago by Keith Brown

Refs #8962 Written unit test for Loader

Test includes cases for unquoted data, quoted data, incorrect columns and blank files

Changeset: 9f3dde75770b20c2495fcd5b47eb7c2bc9ff1143

comment:13 Changed 6 years ago by Keith Brown

Refs #8962 Stich group IDs now start at 1

In order for an unedited stitchID column in the Table to be classed as "in it's own stich group", stitch groups needed to start at 1 rather than 0, so that 0 is classed as unedited.

The Load aglorithm and test have been updated for this.

Changeset: 478ee108d00c058f838b0c6c6ad8d21ecde9a079

comment:14 Changed 6 years ago by Keith Brown

Refs #8962 Updated Load test

One test haddn't been updated when the stich group indexes were changed to start at one

Changeset: c1c8a32c484010dca8f9f920d6f416584981a49e

comment:15 Changed 6 years ago by Keith Brown

Refs #8962 LoadReflTBL tidied up

Removed unneeded headers

Streaming to the row object is now in a count controlled loop

Call to the logger just before throw removed

Some Copyright stuff removed from header

Changeset: fc0dead2f22f8f00b74614c15ff48dce70b0cf6c

comment:16 Changed 6 years ago by Keith Brown

Refs #8962 Save Algorithm written

Save algorithm now saves in the same format as the original python function, however this doesn't save a bunch of trailing lines of commas to the file.

Changeset: 4bb44ab0a6a3f4e1ec57501dbe67729406655bd9

comment:17 Changed 6 years ago by Keith Brown

Refs #8962 Unit test for SaveReflTBL

Unit test has been written and tests data with and without commas, failing on a group too large, failing when the tableworkspace isn't wide enough, and also that the files can be Loaded with LoadReflTBL.

Changeset: 2d45d123976c7596d47532baa51c24f311de5cfd

comment:18 Changed 6 years ago by Keith Brown

Refs #8962 Added wiki and documentation

Added missing Wiki info and fixed some docuamtation so that doxygen will find it

Changeset: ec2f3a970eb1e9da110e052ff42295e4232479b3

comment:19 Changed 6 years ago by Keith Brown

Merge branch origin/master into 'feature/8962_Save_Load_Refl_tbl'

Conflicts:

Code/Mantid/Framework/Kernel/src/LogParser.cpp

Refs #8962

Changeset: 4bc42405f866caa2f84b5fe7dbd83d673f7e424f

comment:20 Changed 6 years ago by Keith Brown

Refs #8962 ifstream constructor needed c strings

the mac buildserver fialed due to my ifstream constructors being passed std::strings, when they wanted c strings.

Changeset: 4bb03ec569b0b8ed6276d9cfbb68a26a247529e9

comment:21 Changed 6 years ago by Keith Brown

Refs #8962 ofstream constructor needed c strings

the mac buildserver fialed due to my ofstream constructors being passed std::strings, when they wanted c strings.

Changeset: b407634478faf3dd71edbd960c7626af0ea43531

comment:22 Changed 6 years ago by Russell Taylor

comment:24 Changed 6 years ago by Keith Brown

Refs #8962 Fixing cppcheck and gcc warnings

this should fix the cppcheck and gcc compiler warnings caused by my work on this ticket

Changeset: 94d9fb7aaea7093b70f9e58eb90a8b10349736d9

comment:25 Changed 6 years ago by Keith Brown

Refs #8962 Accidently removed a call when fixing errors

I accidently removed a call to the function that counts quotes when i was fixing the warnings. so the load algorithm wouldn't accept a file with quotes

Changeset: 523e2ec2138cb2e75079d0a43e0f2e8ac372dbc2

comment:26 Changed 6 years ago by Keith Brown

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

comment:27 Changed 6 years ago by Keith Brown

To Tester.

Grab INTER_NR_test2.tbl from the ISIS sample Data and feed it into LoadReflTBL.

The way that TBL files are displayed in a tableworkspace is different to the ISIS reflectometry GUI. This is because we're removing the restriction of 3 runs per group.

The 17 columns are split up into rows of 8, so a single row in the TBL file would be split into up to 3 colums like so: (Where Z is the newly created stitch group index)

A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q

becomes

A, B, C, D, E, P, Q, Z
F, G, H, I, J, P, Q, Z
K, L, M, N, O, P, Q, Z
(if a row would be missing data in all of the first 5 columns, it is considered blank and omitted)

Save the resulting workspace using SaveReflTBL and compare the saved file with the original. the two rows should be identical, if they're not in the same order it doesn't matter, only that the data on a single row is the same. You'll notice that the excess commas are gone, this was an improvement to the format.

Make edits to the TBL file (use a plain text editor) and load it back in, test the following:

  • it rejects files that have a line with less than 16 commas
  • it rejects files that have a line the contains more than 16 commas outside of quotes
  • it will consider data in quotes as part of the same cell even if it contains a comma

Then make sure that SaveReflTBL:

  • saves files that can be loaded into the ISIS reflectometry GUI
  • Rejects tableworkspaces with a stitch Id group with 4 memebers or more. You will probably have to alter a tableworkspace in python to do this. (This is a compatibiltiy save format so although we're removing the 3 per group restriction, we need to keep it for formats begin loaded into this gui)

If this is tested while I am out of the office, direct questions to Owen.

comment:28 Changed 6 years ago by Russell Taylor

  • Status changed from verify to reopened
  • Resolution fixed deleted

There are a number of doxygen warnings in LoadReflTBL.cpp that should be addressed.

comment:29 Changed 6 years ago by Keith Brown

  • Status changed from reopened to inprogress

Refs #8962 Couple of Doxygen fixes

There were some doxygen warnings about undocumented parameters and nonexistant parameters. It was a mere case of a couple of uncapitalised Bs.

Changeset: 11b9f30e0f799f5bb6741f3e7afa45eb7dc9bb6d

comment:30 Changed 6 years ago by Keith Brown

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

comment:31 Changed 6 years ago by Owen Arnold

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

comment:32 Changed 6 years ago by Owen Arnold

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/8962_Save_Load_Refl_tbl'

Full changeset: 994a39fd31c8c87e660cd34cf977d2334f441ba5

comment:33 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 9805

Note: See TracTickets for help on using tickets.