-
Notifications
You must be signed in to change notification settings - Fork 6
move detpar to detector arrays #784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,19 +28,36 @@ function test_copy_returns_distinct_object(obj) | |
| sqw_copy = copy(sqw_obj); | ||
|
|
||
| sqw_copy.main_header.title = 'test_copy'; | ||
| sqw_copy = sqw_copy.change_header(struct([])); | ||
| assertFalse(equal_to_tol(sqw_copy.main_header, sqw_obj.main_header)); | ||
|
|
||
| % want to test expinfo/instruments and expinfo/detpar | ||
| % separately, so make 2 copies of expinfo | ||
| expinf1 = sqw_copy.experiment_info; | ||
| expinf2 = sqw_copy.experiment_info; | ||
|
|
||
| % all instruments changed with new name to make expinf different | ||
| for i=1:numel(expinf1.instruments), expinf1.instruments{i}.name = 'copy'; end | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally, I really dislike inline
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. expinf1.instruments = cellfun(@(x) setfield(x, 'name', 'copy'), expinf1.instruments, 'UniformOutput', false)might be cleaner? Though admittedly MATLAB's verbosity in making the output a cell-array this sort of defeats that.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to change the formatting. Arguable if the cellfun version is cleaner - it is very opaque and in the interests of making clear what a test does if there is the problem here I would retain the for loop. |
||
| sqw_copy = sqw_copy.change_header(expinf1); | ||
| assertFalse(equal_to_tol(sqw_copy.experiment_info, sqw_obj.experiment_info)); | ||
|
|
||
| % all detpar detectors changed with new azim value to make expinf different | ||
| for i=1:numel(expinf2.detector_arrays), expinf2.detector_arrays(i).azim = 0; end | ||
| sqw_copy = sqw_copy.change_header(expinf2); | ||
| assertFalse(equal_to_tol(sqw_copy.my_detpar(), sqw_obj.my_detpar())); | ||
| %{ | ||
| % old version of detpar change with separate detpar left for | ||
| % reference to see what was intended | ||
| dtp = sqw_copy.my_detpar(); | ||
| dtp.azim(1:10) = 0; | ||
| sqw_copy = sqw_copy.change_detpar(dtp); | ||
| sqw_copy.data.dax = [2, 1]; | ||
| sqw_copy.data.pix.signal = 1; | ||
| %} | ||
|
|
||
| % changed data is not mirrored in initial | ||
| assertFalse(equal_to_tol(sqw_copy.main_header, sqw_obj.main_header)); | ||
| assertFalse(equal_to_tol(sqw_copy.experiment_info, sqw_obj.experiment_info)); | ||
| assertFalse(equal_to_tol(sqw_copy.my_detpar(), sqw_obj.my_detpar())); | ||
| sqw_copy.data.dax = [2, 1]; | ||
| sqw_copy.data.pix.signal = 1; | ||
| assertFalse(equal_to_tol(sqw_copy.data, sqw_obj.data)); | ||
| assertFalse(equal_to_tol(sqw_copy.data.pix, sqw_obj.data.pix)); | ||
|
|
||
| end | ||
|
|
||
| function test_copy_excluding_pix_returns_empty_pix_data(obj) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
|
|
||
| properties(Access=private) | ||
| instruments_ = {}; %IX_inst.empty; | ||
| detector_arrays_ = [] | ||
| detector_arrays_ = IX_detector_array.empty; | ||
| samples_ = {}; % IX_samp.empty; | ||
| expdata_ = IX_experiment(); | ||
| end | ||
|
|
@@ -19,6 +19,7 @@ | |
| properties(Dependent,Hidden) | ||
| % property providing compartibility with old header interface | ||
| header | ||
| detpar | ||
| end | ||
| properties(Constant,Access=private) | ||
| fields_to_save_ = {'instruments','detector_arrays','samples','expdata'}; | ||
|
|
@@ -90,6 +91,10 @@ | |
| 'Must give all of detector_array, instrument and sample or the structure representing them') | ||
| end | ||
| end | ||
|
|
||
| function add_detector_array(obj,detarr) | ||
| obj.detector_arrays_(end+1) = detarr; | ||
| end | ||
| % | ||
| function oldhdrs = convert_to_old_headers(obj,header_num) | ||
| % convert Experiment into the structure suitable to be | ||
|
|
@@ -295,6 +300,15 @@ | |
| head = [head{:}]; | ||
| head = rmfield(head,{'instrument','sample'}); | ||
| end | ||
|
|
||
| function dp = get.detpar(obj) | ||
| if numel(obj.detector_arrays)>0 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this not equivalent to |
||
| dp = obj.detector_arrays(1); | ||
| dp = dp.convert_to_old_detpar(); | ||
| else | ||
| dp = struct(); | ||
| end | ||
| end | ||
| end | ||
| methods(Access=protected) | ||
| %------------------------------------------------------------------ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,7 +106,7 @@ | |
|
|
||
| % Construct the new header Experiment object | ||
| % update with expdata, which maybe should go in the Experiment constructor | ||
| obj = Experiment([], instruments, samples); | ||
| obj = Experiment(IX_detector_array.empty, instruments, samples); | ||
| obj.expdata = expdata; | ||
|
Comment on lines
+109
to
110
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would allow old interface (user would not be so keen to learn about |
||
|
|
||
| end % function build_from_old_headers | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,7 +134,8 @@ | |
| % ---------------------------------------------------------------------- | ||
| d.main_header=main_header; | ||
| d.experiment_info=header; | ||
| d.detpar=det0; | ||
| d.experiment_info.detector_arrays(end+1) = IX_detector_array(det0); | ||
| d.detpar=struct([]); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In keeping with other changes would
Comment on lines
+141
to
+142
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would not work without array compression and will be very inefficient, as |
||
| d.data=data_sqw_dnd(sqw_datstr); | ||
| w=sqw(d); | ||
|
|
||
|
|
@@ -187,7 +188,7 @@ | |
| else | ||
| sample = obj.sample; | ||
| end | ||
| header = Experiment([],obj.instrument,sample); | ||
| header = Experiment(IX_detector_array.empty,instrument,sample); | ||
|
|
||
|
|
||
| uoffset = [0;0;0;0]; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,7 +88,11 @@ | |
| wout = noisify(w,varargin); | ||
|
|
||
| function dtp = my_detpar(obj) | ||
| dtp = obj.detpar_x; | ||
| if ~isempty(obj.detpar_x) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If both are empty this will error. Expected?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. obj in this case is a sqw (or array thereof). Guess it is conceivable that obj could be an empty array with the right type. If it errors maybe we rewrite in python? |
||
| dtp = obj.detpar_x; | ||
| elseif ~isempty(obj.experiment_info.detector_arrays) | ||
| dtp = obj.experiment_info.detector_arrays(1).convert_to_old_detpar(); | ||
| end | ||
| end | ||
|
|
||
| function obj = change_detpar(obj,dtp) | ||
|
|
@@ -226,8 +230,19 @@ | |
|
|
||
| methods(Static) | ||
| function obj = loadobj(S) | ||
| % boilerplate loadobj method, calling generic method of | ||
| % modified boilerplate loadobj method, calling generic method of | ||
| % saveable class | ||
| % the generic code is preceded by the conversion of old-style | ||
| % detpar info into the new detector arrays. This allows loading | ||
| % of .mat files without converting them on disk.. | ||
| if isfield(S,'experiment_info') && isfield(S,'detpar') | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just tested it and you can do if isfield(S, {'experimental_info', 'detpar'})
Comment on lines
+269
to
+270
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should all this be moved to |
||
| if ~isempty(S.detpar) && isempty(S.experiment_info.detector_arrays) | ||
| dd = IX_detector_array(S.detpar); | ||
| S.experiment_info.detector_arrays = IX_detector_array.empty; | ||
| S.experiment_info.detector_arrays(end+1) = dd; | ||
| S.detpar = struct([]); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| end | ||
| end | ||
| obj = sqw(); | ||
| obj = loadobj@serializable(S,obj); | ||
| end | ||
|
|
@@ -272,6 +287,18 @@ | |
| end | ||
| ss = rmfield(ss,'header'); | ||
| end | ||
|
|
||
| % assuming that detpar exists and is not empty, convert | ||
| % its contents into a detector array. There are test | ||
| % datasets with empty detpar and those should be left | ||
| % unconverted, they seem to be testing other items in | ||
| % the sqw and ignoring the detpar. | ||
| if isfield(ss,'experiment_info') && isfield(ss,'detpar') && ~isempty(ss.detpar) | ||
| dd = IX_detector_array(ss.detpar); | ||
| ss.experiment_info.detector_arrays(end+1) = dd; | ||
| ss = rmfield(ss,'detpar'); | ||
| end | ||
|
|
||
| if isfield(ss,'data_') | ||
| ss.data = ss.data_; | ||
| ss = rmfield(ss,'data_'); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -253,7 +253,8 @@ function check_obj_initated_properly(obj) | |
| function check_error_report_fail_(obj,pos_mess) | ||
| % check if error occured during io operation and throw if it does happened | ||
| [mess,res] = ferror(obj.file_id_); | ||
| if res ~= 0; error('SQW_FILE_IO:io_error',... | ||
| if res ~= 0 | ||
| error('SQW_FILE_IO:io_error',... | ||
|
||
| '%s -- Reason: %s',pos_mess,mess); | ||
| end | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,7 +72,14 @@ | |
| % Get detector parameters | ||
| % ----------------------- | ||
| if ~(opts.head||opts.his) | ||
| sqw_struc.detpar = obj.get_detpar(); | ||
| dp = obj.get_detpar(); | ||
| if isa(headers, 'Experiment') | ||
| dp = obj.get_detpar(); | ||
| headers.detector_arrays(end+1) = IX_detector_array(dp); | ||
| sqw_struc.detpar = struct([]); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| else | ||
| sqw_struc.detpar = obj.get_detpar(); | ||
| end | ||
| end | ||
|
|
||
| % Get data | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why to change interface in such convoluted way?
why not to do
sqw_obj.experiment_info.idand wire up this property to the array internally