Back to the main page.
Bug 3464 - ft_volumenormalise suppresses further warnings
| Status | CLOSED FIXED |
| Reported | 2018-10-29 11:26:00 +0100 |
| Modified | 2019-08-10 12:37:09 +0200 |
| Product: | FieldTrip |
| Component: | forward |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Linux |
| Importance: | P5 minor |
| Assigned to: | Robert Oostenveld |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Lau Møller Andersen - 2018-10-29 11:26:38 +0100
On line 194 of ft_volumenormalise, warnings are shut off to never be put on again. I propose to fix this by adding ft_warning('on') at the end of the script
Robert Oostenveld - 2018-10-31 09:19:18 +0100
they should be switched back on with this on line 383 % restore the previous warning state warning(ws); but it should be ft_warning instead of warning.
Robert Oostenveld - 2018-10-31 09:28:51 +0100
this also applies to mac011> grep 'warning(ws)' *.m ft_clusterplot.m:ft_warning(ws); ft_statistics_montecarlo.m:warning(ws); % revert to original state ft_volumenormalise.m:warning(ws); Please search the subdirectories as well. Note that in "external" the ft_warning function should not be used.
Robert Oostenveld - 2018-10-31 09:31:17 +0100
please do grep 'warning(ws)' *.m grep 'warning(ws)' */*.m grep 'warning(ws)' */*/*.m grep 'warning(ws)' */*/*/*.m
Lau Møller Andersen - 2018-10-31 10:56:17 +0100
is it only warning(ws) that should be changed to ft_warning(ws) or is it all warnings that use warning that should be changed to ft_warning? I am specifically thinking about the ft_notification functions that seem to do something with the lowlevel Matlab warning function?
Robert Oostenveld - 2018-10-31 21:09:15 +0100
(In reply to Lau Møller Andersen from comment #4) if ft_warning is used in the functions, and ws=ft_warning('off') is used, then ft_warning(ws) should be used further down.
Lau Møller Andersen - 2018-11-01 10:06:52 +0100
if ft_warning is used in the functions, and ws=ft_warning('off') is used, then ft_warning(ws) should be used further down. Ok, got that.! The other way around then; if ws = warning('off', 'some_warning') and warning(ws) exists at the end, should they both be changed to ft_warning, or should nothing be changed the
Robert Oostenveld - 2018-11-01 15:39:45 +0100
I think that changing "warning" into "ft_warning" everywhere is out of scope here. It is just about getting them consistent within each function. I found this https://unix.stackexchange.com/questions/68138/use-grep-to-find-all-files-in-a-directory-with-two-strings and https://linuxgazette.net/152/misc/lg/searching_for_multiple_strings_patterns_with_grep.html, which resulted in for FILE in `find . -name \*.m` do grep -q ft_warning $FILE && grep -q "^warning(\| warning(" $FILE && echo $FILE done and that returns ./inverse/private/ft_notification.m ./inverse/private/hasyokogawa.m ./compat/obsolete/ft_prepare_concentricspheres.m ./compat/obsolete/ft_sensorrealign.m ./ft_singleplotTFR.m ./test/test_bug2732.m ./fileio/ft_read_event.m ./fileio/ft_read_header.m ./fileio/private/ft_notification.m ./fileio/private/read_ctf_mri4.m ./fileio/private/hasyokogawa.m ./fileio/private/read_ctf_mri.m ./fileio/private/fixpos.m ./connectivity/private/ft_notification.m ./ft_statistics_montecarlo.m ./preproc/private/ft_notification.m ./ft_volumenormalise.m ./utilities/hasyokogawa.m ./utilities/private/ft_notification.m ./utilities/private/fixpos.m ./private/warp_dykstra2012.m ./private/fixpos.m ./plotting/ft_plot_text.m ./plotting/ft_plot_dipole.m ./plotting/ft_plot_vector.m ./plotting/ft_plot_topo.m ./plotting/ft_plot_mesh.m ./plotting/ft_plot_sens.m ./plotting/private/ft_notification.m ./plotting/private/fixpos.m ./plotting/ft_plot_vol.m ./plotting/ft_plot_slice.m ./statfun/private/ft_notification.m ./specest/private/ft_notification.m ./external/artinis/ft_nirs_prepare_ODtransformation.m ./forward/private/ft_notification.m ./forward/private/hasyokogawa.m ./forward/private/fixpos.m ./ft_multiplotTFR.m Some of them should not be changed, but most actually have the same bug as ft_volumenormalise. Given the extent of the problem, I suggest that I make the changes. I can run the fieldtrip/test scripts prior to committing it to github.
Robert Oostenveld - 2018-11-01 16:22:19 +0100
I just noticed that the warning state returned by ft_warning was the one after changing , which means it cannot be changed back. I also fixed that. roboos@mentat001> git commit -a [bug3464 bdd07f9] FIX bug3464 - do not mix warning and ft_warning, return the warning state prior to changing it 30 files changed, 64 insertions(+), 65 deletions(-) roboos@mentat001> git push --set-upstream origin bug3464 Warning: untrusted X11 forwarding setup failed: xauth key data not generated Counting objects: 103, done. Delta compression using up to 8 threads. Compressing objects: 100% (47/47), done. Writing objects: 100% (61/61), 7.08 KiB | 0 bytes/s, done. Total 61 (delta 55), reused 19 (delta 14) remote: Resolving deltas: 100% (55/55), completed with 42 local objects. remote: remote: Create a pull request for 'bug3464' on GitHub by visiting: remote: https://github.com/robertoostenveld/fieldtrip/pull/new/bug3464 remote: To git@github.com:robertoostenveld/fieldtrip.git * [new branch] bug3464 -> bug3464 Branch bug3464 set up to track remote branch bug3464 from origin.
Robert Oostenveld - 2018-11-01 16:24:17 +0100
I merged the changes, see https://github.com/fieldtrip/fieldtrip/pull/866