octave-dev

[OctDev] Dicom package / isdicom function

[adam a. <ada...@ho...>]

Hi,

I've written isdicom (as an .m file), a function which is currently missing from the Dicom package.  If it's likely to be useful then I'm happy to submit it to Octave-forge (is there a different mailing list for Octave-forge?), or I could forward the code to the maintainer of the package if that's more helpful.

Adam
 


 		 	   		  

Re: [OctDev] Dicom package / isdicom function

[Andy B. <and...@gm...>]

On 13 September 2012 12:24, adam aitkenhead <ada...@ho...> wrote:
>
> Hi,
>
> I've written isdicom (as an .m file), a function which is currently missing
> from the Dicom package.  If it's likely to be useful then I'm happy to
> submit it to Octave-forge (is there a different mailing list for
> Octave-forge?), or I could forward the code to the maintainer of the package
> if that's more helpful.
>
> Adam

If you email to me, I will take a look, and commit it to the svn repo.
Are you OK with GPL V3 or higher? Do you want your name in the
comments at the top of the file?

Thanks,

Andy (dicom package maintainer)

Re: [OctDev] Dicom package / isdicom function

[Andy B. <and...@gm...>]

>> Date: Thu, 13 Sep 2012 14:57:29 +0100
>
>> Subject: Re: [OctDev] Dicom package / isdicom function
>> From: and...@gm...
>> To: ada...@ho...
>>
>> > I have access to Matlab too, and it recognises the non-standard DICOM
>> > file.
>> > (Just to note, although I have tested the ML behaviour for a some test
>> > files, I didn't use Matlab as a basis for the code.)
>>
>> Excellent. That is the way to do it. We are careful about copyright.
>>
>> Andy
On 19 September 2012 17:26, adam aitkenhead <ada...@ho...> wrote:
>
> Hi Andy,
>
> I've attached an updated version of the isdicom function which can now check
> a list of files (in a cell array) in one go, which is much quicker than
> checking each file separately.  Again, no rush for releasing a new version
> of the toolbox - just some changes I was making for my own code anyway.
>
> Also on a different note, I've written functions which read/write the
> Analyze format, giving functions equivalent to Matlab's analyze75info and
> analyze75read.  Would you rather keep the Dicom toolbox purely for the Dicom
> format, or are you interested in expanding it to become a general Medical
> File Format toolbox?  No worries if not, just thought I'd see what you
> thought before I see where else they could fit into Octave-forge.
>
> Adam

Note that I have included the list for further discussion, and edited
older text to chronological order.

I have not used analyze75. If it is just m-files, then the image
package is probably a good place. The main reason dicom is on its own
is that it has burdensome dependencies.

-- 
/* andy buckle */

Re: [OctDev] Dicom package / isdicom function

[Kris T. <kri...@gm...>]

> From: Andy Buckle 
> Sent: 19 September 2012 19:01
> >
> > From adamaitkenhead

> > Also on a different note, I've written functions which read/write the
> > Analyze format, giving functions equivalent to Matlab's analyze75info
and
> > analyze75read.  Would you rather keep the Dicom toolbox purely for the
> Dicom
> > format, or are you interested in expanding it to become a general
Medical
> > File Format toolbox?  No worries if not, just thought I'd see what you
> > thought before I see where else they could fit into Octave-forge.
> >
> > Adam
> 
> I have not used analyze75. If it is just m-files, then the image
> package is probably a good place. The main reason dicom is on its own
> is that it has burdensome dependencies.
> 

I'd keep it out of the dicom package as well. 

Note that there's a number of packages out there to read nifti files (which
is a superset of analyze75, see http://nifti.nimh.nih.gov/nifti-1/), not all
with very clear license though. One that seems ok is
http://www.nitrc.org/projects/cbinifti/, but I haven't used it (it might
rely on unix pipes) . Then there's http://niftilib.sourceforge.net/ which
also provides matlab support, but as it's the IO basis for SPM, and a lot of
SPM works on Octave, it might work.

Of course, there's still a place for analyze75info etc compatible functions.
I have no idea if that already exists somewhere else.

Kris

Re: [OctDev] Dicom package / isdicom function

[Carnë D. <car...@gm...>]

On 19 September 2012 20:00, Andy Buckle <and...@gm...> wrote:
>>> Date: Thu, 13 Sep 2012 14:57:29 +0100
>>
>>> Subject: Re: [OctDev] Dicom package / isdicom function
>>> From: and...@gm...
>>> To: ada...@ho...
>>>
>>> > I have access to Matlab too, and it recognises the non-standard DICOM
>>> > file.
>>> > (Just to note, although I have tested the ML behaviour for a some test
>>> > files, I didn't use Matlab as a basis for the code.)
>>>
>>> Excellent. That is the way to do it. We are careful about copyright.
>>>
>>> Andy
> On 19 September 2012 17:26, adam aitkenhead <ada...@ho...> wrote:
>>
>> Hi Andy,
>>
>> I've attached an updated version of the isdicom function which can now check
>> a list of files (in a cell array) in one go, which is much quicker than
>> checking each file separately.  Again, no rush for releasing a new version
>> of the toolbox - just some changes I was making for my own code anyway.
>>
>> Also on a different note, I've written functions which read/write the
>> Analyze format, giving functions equivalent to Matlab's analyze75info and
>> analyze75read.  Would you rather keep the Dicom toolbox purely for the Dicom
>> format, or are you interested in expanding it to become a general Medical
>> File Format toolbox?  No worries if not, just thought I'd see what you
>> thought before I see where else they could fit into Octave-forge.
>>
>> Adam
>
> Note that I have included the list for further discussion, and edited
> older text to chronological order.
>
> I have not used analyze75. If it is just m-files, then the image
> package is probably a good place. The main reason dicom is on its own
> is that it has burdensome dependencies.

I agree with Andy. The analyze75read/write functions would fit better
on the image package. Are their API matlab compatible?

Carnë

Re: [OctDev] Dicom package / isdicom function

[adam a. <ada...@ho...>]

Hi all,
I see what you mean about the dicom package's dependencies, sounds like the image package is a better place for it.
The underlying read function has a different calling syntax from Matlab, as it reads both the header and the image in one go (I wrote it like that before I realized there were similar functions in Matlab).  However I've also written functions which call it using the same format as analyze75info and analyze75read.  This does mean that it runs slower than Matlab, but it shouldn't be difficult to change it to read only the appropriate parts if needed.
I've only been able to test it with my own Analyze files, so it's not been tested using other files with different orientations or anything like that.
Adam

 


> From: car...@gm...
> Date: Thu, 20 Sep 2012 12:33:35 +0200
> Subject: Re: [OctDev] Dicom package / isdicom function
> To: ada...@ho...
> CC: and...@gm...; oct...@li...
> 
> On 19 September 2012 20:00, Andy Buckle <and...@gm...> wrote:
> >>> Date: Thu, 13 Sep 2012 14:57:29 +0100
> >>
> >>> Subject: Re: [OctDev] Dicom package / isdicom function
> >>> From: and...@gm...
> >>> To: ada...@ho...
> >>>
> >>> > I have access to Matlab too, and it recognises the non-standard DICOM
> >>> > file.
> >>> > (Just to note, although I have tested the ML behaviour for a some test
> >>> > files, I didn't use Matlab as a basis for the code.)
> >>>
> >>> Excellent. That is the way to do it. We are careful about copyright.
> >>>
> >>> Andy
> > On 19 September 2012 17:26, adam aitkenhead <ada...@ho...> wrote:
> >>
> >> Hi Andy,
> >>
> >> I've attached an updated version of the isdicom function which can now check
> >> a list of files (in a cell array) in one go, which is much quicker than
> >> checking each file separately.  Again, no rush for releasing a new version
> >> of the toolbox - just some changes I was making for my own code anyway.
> >>
> >> Also on a different note, I've written functions which read/write the
> >> Analyze format, giving functions equivalent to Matlab's analyze75info and
> >> analyze75read.  Would you rather keep the Dicom toolbox purely for the Dicom
> >> format, or are you interested in expanding it to become a general Medical
> >> File Format toolbox?  No worries if not, just thought I'd see what you
> >> thought before I see where else they could fit into Octave-forge.
> >>
> >> Adam
> >
> > Note that I have included the list for further discussion, and edited
> > older text to chronological order.
> >
> > I have not used analyze75. If it is just m-files, then the image
> > package is probably a good place. The main reason dicom is on its own
> > is that it has burdensome dependencies.
> 
> I agree with Andy. The analyze75read/write functions would fit better
> on the image package. Are their API matlab compatible?
> 
> Carnë
 		 	   		  

Re: [OctDev] Dicom package / isdicom function

[Carnë D. <car...@gm...>]

On 21 September 2012 10:18, adam aitkenhead <ada...@ho...> wrote:
> The underlying read function has a different calling syntax from Matlab, as
> it reads both the header and the image in one go (I wrote it like that
> before I realized there were similar functions in Matlab).  However I've
> also written functions which call it using the same format as analyze75info
> and analyze75read.  This does mean that it runs slower than Matlab, but it
> shouldn't be difficult to change it to read only the appropriate parts if
> needed.

It's ok to do more than the matlab functions, there's plenty of
examples where that is the case. It's just not very good to have
conflicts with their API. So if your analyze75read works as in
"[image, header] = analyze75read (fname)", it's still matlab
compatible while being more useful at the same time. It seems to me
that matlab also has a analyze75info function, maybe you'd like to
make it compatible? I'm planning on making a release of the image
package Sunday in case you are interested in having your functions
included now. Otherwise they can still be included on the next
release.

Carnë

Re: [OctDev] Dicom package / isdicom function

[adam a. <ada...@ho...>]

I probably won't have a chance to send it before Sunday (the files are on my laptop which I don't have with me this weekend).  But I should be able to send it next week, so the following release should be no problem.Thanks,Adam

 


> From: car...@gm...
> Date: Fri, 21 Sep 2012 13:53:56 +0200
> Subject: Re: [OctDev] Dicom package / isdicom function
> To: ada...@ho...
> CC: and...@gm...; oct...@li...
> 
> On 21 September 2012 10:18, adam aitkenhead <ada...@ho...> wrote:
> > The underlying read function has a different calling syntax from Matlab, as
> > it reads both the header and the image in one go (I wrote it like that
> > before I realized there were similar functions in Matlab).  However I've
> > also written functions which call it using the same format as analyze75info
> > and analyze75read.  This does mean that it runs slower than Matlab, but it
> > shouldn't be difficult to change it to read only the appropriate parts if
> > needed.
> 
> It's ok to do more than the matlab functions, there's plenty of
> examples where that is the case. It's just not very good to have
> conflicts with their API. So if your analyze75read works as in
> "[image, header] = analyze75read (fname)", it's still matlab
> compatible while being more useful at the same time. It seems to me
> that matlab also has a analyze75info function, maybe you'd like to
> make it compatible? I'm planning on making a release of the image
> package Sunday in case you are interested in having your functions
> included now. Otherwise they can still be included on the next
> release.
> 
> Carnë
 		 	   		  

Re: [OctDev] Dicom package / isdicom function

[Carnë D. <car...@gm...>]

On 21 September 2012 16:10, adam aitkenhead <ada...@ho...> wrote:
>> From: car...@gm...
>> Date: Fri, 21 Sep 2012 13:53:56 +0200
>>
>> On 21 September 2012 10:18, adam aitkenhead <ada...@ho...>
>> wrote:
>> > The underlying read function has a different calling syntax from Matlab,
>> > as
>> > it reads both the header and the image in one go (I wrote it like that
>> > before I realized there were similar functions in Matlab). However I've
>> > also written functions which call it using the same format as
>> > analyze75info
>> > and analyze75read. This does mean that it runs slower than Matlab, but
>> > it
>> > shouldn't be difficult to change it to read only the appropriate parts
>> > if
>> > needed.
>>
>> It's ok to do more than the matlab functions, there's plenty of
>> examples where that is the case. It's just not very good to have
>> conflicts with their API. So if your analyze75read works as in
>> "[image, header] = analyze75read (fname)", it's still matlab
>> compatible while being more useful at the same time. It seems to me
>> that matlab also has a analyze75info function, maybe you'd like to
>> make it compatible? I'm planning on making a release of the image
>> package Sunday in case you are interested in having your functions
>> included now. Otherwise they can still be included on the next
>> release.
>>
>> Carnë
>
> I probably won't have a chance to send it before Sunday (the files are on my
> laptop which I don't have with me this weekend).  But I should be able to
> send it next week, so the following release should be no problem.
> Thanks,
> Adam

This is taking me more time than initially expected. If you want,
could still try to add them to this release.

Carnë

Re: [OctDev] Analyze75

[Carnë D. <car...@gm...>]

On 25 September 2012 11:17, adam aitkenhead <ada...@ho...> wrote:
> Hi Carnë,
>
> Attached are those m files for analyze75info and analyze75read - I've
> reformatted them to use Matlab's syntax.  I've still to tidy up
> analyze75write, but I'll send it on later this week if I can.
>
> Adam

Hi Adam

Thank you for the functions. I have just made the commit and added
them to the package. I have added your e-mail to the copyright notice
but let me know if you have another address that you prefer. I can
wait a bit more to have the whole analyze75 set of functions.

If you are already tidying up the next function, would be cool if you
could follow some of the standards of Octave core (most in Octave
Forge don't follow it but I find they good standards). Here's some:

  * indent the function body (this will save from writing end %function)
  * spaces after function names
  * no spaces after variables for indexing
  * user @var for variables name in the help text (do not capitalize them)

I'd suggest the others but it seems to me that you want to keep your
code Matlab compatible. Is that correct?

Carnë

Re: [OctDev] Analyze75

[adam a. <ada...@ho...>]

Hi Carnë

Thanks for the tips, they all sound like a good idea - I'll aim to follow them for future code.  (Yup, I've been trying to keep the code so that it also works in Matlab where possible.)

When you're ready with everything else just go ahead and release the package, I can't guarantee I'll be able to tidy up the remaining function quickly so I don't want to hold things up.

This email address is fine, thanks for adding it.

Thanks,
Adam
 


> From: car...@gm...
> Date: Tue, 25 Sep 2012 14:51:42 +0200
> Subject: Re: [OctDev] Analyze75
> To: ada...@ho...
> CC: oct...@li...
> 
> On 25 September 2012 11:17, adam aitkenhead <ada...@ho...> wrote:
> > Hi Carnë,
> >
> > Attached are those m files for analyze75info and analyze75read - I've
> > reformatted them to use Matlab's syntax.  I've still to tidy up
> > analyze75write, but I'll send it on later this week if I can.
> >
> > Adam
> 
> Hi Adam
> 
> Thank you for the functions. I have just made the commit and added
> them to the package. I have added your e-mail to the copyright notice
> but let me know if you have another address that you prefer. I can
> wait a bit more to have the whole analyze75 set of functions.
> 
> If you are already tidying up the next function, would be cool if you
> could follow some of the standards of Octave core (most in Octave
> Forge don't follow it but I find they good standards). Here's some:
> 
>   * indent the function body (this will save from writing end %function)
>   * spaces after function names
>   * no spaces after variables for indexing
>   * user @var for variables name in the help text (do not capitalize them)
> 
> I'd suggest the others but it seems to me that you want to keep your
> code Matlab compatible. Is that correct?
> 
> Carnë
 		 	   		  

Re: [OctDev] Analyze75

[Carnë D. <car...@gm...>]

On 25 September 2012 15:11, adam aitkenhead <ada...@ho...> wrote:
> Thanks for the tips, they all sound like a good idea - I'll aim to follow
> them for future code.  (Yup, I've been trying to keep the code so that it
> also works in Matlab where possible.)

I've been changing a bit the input check. The standard we have is to
check the value of nargin. Doing this at the start means that you
don't have to check if the variable exists later.

Another thing I noticed is the many ways it tries to handle invalid
filenames. While it looks kinda slick, I'm not sure it's a good idea
to have the function doing it, it should be done by the script that
calls it. When you give wrong arguments to a function, trying to guess
what is right can lead to weird results. Or does matlab also behaves
that way (it's not documented at least). My suggestion would be to
maybe display the file selection if no argument is given but return an
error if the filenames are not valid files. What do you think?

Carnë

Re: [OctDev] Analyze75

[adam a. <ada...@ho...>]

All sounds good to me.  Makes sense to keep to the standard way of doing things to make it behave as others might expect.  I'm not all that familiar with the standard (yet), so if you see any ways to standardise it that is great.

About the input checks - I'd aimed to make it behave as Matlab does for all acceptable inputs.  But for the cases where Matlab throws an error, this function generally asks the user what to do instead.  But I'm happy for this to be simplified or left for other functions which call these.

 
> From: car...@gm...
> Date: Tue, 25 Sep 2012 16:30:48 +0200
> Subject: Re: [OctDev] Analyze75
> To: ada...@ho...
> CC: oct...@li...
> 
> I've been changing a bit the input check. The standard we have is to
> check the value of nargin. Doing this at the start means that you
> don't have to check if the variable exists later.
> 
> Another thing I noticed is the many ways it tries to handle invalid
> filenames. While it looks kinda slick, I'm not sure it's a good idea
> to have the function doing it, it should be done by the script that
> calls it. When you give wrong arguments to a function, trying to guess
> what is right can lead to weird results. Or does matlab also behaves
> that way (it's not documented at least). My suggestion would be to
> maybe display the file selection if no argument is given but return an
> error if the filenames are not valid files. What do you think?
> 
> Carnë
 		 	   		  

Re: [OctDev] Analyze75

[Carnë D. <car...@gm...>]

hey, please avoid top posting. Reply at the bottom

On 25 September 2012 17:07, adam aitkenhead <ada...@ho...> wrote:
>> From: car...@gm...
>> Date: Tue, 25 Sep 2012 16:30:48 +0200
>
>> Another thing I noticed is the many ways it tries to handle invalid
>> filenames. While it looks kinda slick, I'm not sure it's a good idea
>> to have the function doing it, it should be done by the script that
>> calls it. When you give wrong arguments to a function, trying to guess
>> what is right can lead to weird results. Or does matlab also behaves
>> that way (it's not documented at least). My suggestion would be to
>> maybe display the file selection if no argument is given but return an
>> error if the filenames are not valid files. What do you think?
>>
>
> All sounds good to me.  Makes sense to keep to the standard way of doing
> things to make it behave as others might expect.  I'm not all that familiar
> with the standard (yet), so if you see any ways to standardise it that is
> great.

I've made a few more changes. Could you please give them a try to make
sure I didn't broke anything? :p

I've moved the code that was common between read and info into a
shared private function, checks after fseek and fopen, everything in
one function (I see no need to have subfunctions since they are only
called once anyway), replaced a long if block for a switch block,
changed many checks for simpler, used dir to check the file size
(rather than the low level fseek and ftell), and a lot of whitespace
to make it pretty and extended documentation. I tried to make the code
run in matlab (though I'd guess a matlab user would actually just use
the matlab function).

Here's the files:
https://sourceforge.net/p/octave/code/11114/tree//trunk/octave-forge/main/image/inst/private/analyze75filename.m
https://sourceforge.net/p/octave/code/11114/tree//trunk/octave-forge/main/image/inst/analyze75info.m
https://sourceforge.net/p/octave/code/11114/tree//trunk/octave-forge/main/image/inst/analyze75read.m

> About the input checks - I'd aimed to make it behave as Matlab does for all
> acceptable inputs.  But for the cases where Matlab throws an error, this
> function generally asks the user what to do instead.  But I'm happy for this
> to be simplified or left for other functions which call these.

Done.

Carnë

Re: [OctDev] Analyze75

[Carnë D. <car...@gm...>]

On 26 September 2012 11:10, adam aitkenhead <ada...@ho...> wrote:
>> From: car...@gm...
>> Date: Tue, 25 Sep 2012 19:34:15 +0200
>> Subject: Re: [OctDev] Analyze75
>> To: ada...@ho...
>> CC: oct...@li...
>>
>> hey, please avoid top posting. Reply at the bottom
>>
>> On 25 September 2012 17:07, adam aitkenhead <ada...@ho...>
>> wrote:
>> >> From: car...@gm...
>> >> Date: Tue, 25 Sep 2012 16:30:48 +0200
>> >
>> >> Another thing I noticed is the many ways it tries to handle invalid
>> >> filenames. While it looks kinda slick, I'm not sure it's a good idea
>> >> to have the function doing it, it should be done by the script that
>> >> calls it. When you give wrong arguments to a function, trying to guess
>> >> what is right can lead to weird results. Or does matlab also behaves
>> >> that way (it's not documented at least). My suggestion would be to
>> >> maybe display the file selection if no argument is given but return an
>> >> error if the filenames are not valid files. What do you think?
>> >>
>> >
>> > All sounds good to me. Makes sense to keep to the standard way of doing
>> > things to make it behave as others might expect. I'm not all that
>> > familiar
>> > with the standard (yet), so if you see any ways to standardise it that
>> > is
>> > great.
>>
>> I've made a few more changes. Could you please give them a try to make
>> sure I didn't broke anything? :p
>>
>> I've moved the code that was common between read and info into a
>> shared private function, checks after fseek and fopen, everything in
>> one function (I see no need to have subfunctions since they are only
>> called once anyway), replaced a long if block for a switch block,
>> changed many checks for simpler, used dir to check the file size
>> (rather than the low level fseek and ftell), and a lot of whitespace
>> to make it pretty and extended documentation. I tried to make the code
>> run in matlab (though I'd guess a matlab user would actually just use
>> the matlab function).
>>
>> Here's the files:
>>
>> https://sourceforge.net/p/octave/code/11114/tree//trunk/octave-forge/main/image/inst/private/analyze75filename.m
>>
>> https://sourceforge.net/p/octave/code/11114/tree//trunk/octave-forge/main/image/inst/analyze75info.m
>>
>> https://sourceforge.net/p/octave/code/11114/tree//trunk/octave-forge/main/image/inst/analyze75read.m
>>
>> > About the input checks - I'd aimed to make it behave as Matlab does for
>> > all
>> > acceptable inputs. But for the cases where Matlab throws an error, this
>> > function generally asks the user what to do instead. But I'm happy for
>> > this
>> > to be simplified or left for other functions which call these.
>>
>> Done.
>>
>> Carnë
>
> Changes look good.  Few minor edits to analyze75filename.m (see below), but
> otherwise everything works well.  Thanks for the help tidying it up!
>
> -  function filename = analyze75check (filename)
> +  function filename = analyze75filename (filename)
>
> -  elseif (~exist (filename,'file')
> +  elseif (~exist (filename,'file'))
>
> -  error ('analyze75info: no file `%s''', filename)
> +  error ('analyze75: no file `%s''', filename)

Thanks. Fixed

Re: [OctDev] Analyze75

[adam a. <ada...@ho...>]

Hi Carnë,
I've just submitted analyze75write.m to SVN.
Adam 		 	   		  

Re: [OctDev] Analyze75

[Carnë D. <car...@gm...>]

On 1 October 2012 22:30, adam aitkenhead <ada...@ho...> wrote:
> I've just submitted analyze75write.m to SVN.

Nice, thank you. The code looks nice. I have added the function to the
INDEX and NEWS file, and to the see also of the other functions.

Some tips, you don't need to check the value returned by isfield. If
there is a field with that name, it will return true so "if (isfield
(x, name))" is enough, no need to "if (isfield (x, name) == 1).

It would be good practice to also have analyze75filename deal with the
filename part, just as it's doing for the other 2 functions. I'd guess
analyze75filename would take an extra argument ("write", "info" or
"read") for the part of checking if the file already exists. This
would also allow to make it return the right error message with:

      error ('analyze75%s: error message.', analyze75function)

Carnë

Re: [OctDev] Analyze75

[adam a. <ada...@ho...>]

> From: car...@gm...
> Date: Tue, 2 Oct 2012 09:33:17 +0200
> Subject: Re: [OctDev] Analyze75
> To: ada...@ho...
> CC: oct...@li...
> 
> On 1 October 2012 22:30, adam aitkenhead <ada...@ho...> wrote:
> > I've just submitted analyze75write.m to SVN.
> 
> Nice, thank you. The code looks nice. I have added the function to the
> INDEX and NEWS file, and to the see also of the other functions.
> 
> Some tips, you don't need to check the value returned by isfield. If
> there is a field with that name, it will return true so "if (isfield
> (x, name))" is enough, no need to "if (isfield (x, name) == 1).
> 
> It would be good practice to also have analyze75filename deal with the
> filename part, just as it's doing for the other 2 functions. I'd guess
> analyze75filename would take an extra argument ("write", "info" or
> "read") for the part of checking if the file already exists. This
> would also allow to make it return the right error message with:
> 
> error ('analyze75%s: error message.', analyze75function)
> 
> Carnë

Those suggestions sound good, I can make those changes although I probably won't have time before the weekend.  If you're ready for the release of the image package then don't wait, as it won't affect functionality.
 
Thanks,
Adam 		 	   		  

Re: [OctDev] Analyze75

[Carnë D. <car...@gm...>]

On 2 October 2012 11:11, adam aitkenhead <ada...@ho...> wrote:
>> From: car...@gm...
>> Date: Tue, 2 Oct 2012 09:33:17 +0200
>> Subject: Re: [OctDev] Analyze75
>> To: ada...@ho...
>> CC: oct...@li...
>>
>> On 1 October 2012 22:30, adam aitkenhead <ada...@ho...>
>> wrote:
>> > I've just submitted analyze75write.m to SVN.
>>
>> Nice, thank you. The code looks nice. I have added the function to the
>> INDEX and NEWS file, and to the see also of the other functions.
>>
>> Some tips, you don't need to check the value returned by isfield. If
>> there is a field with that name, it will return true so "if (isfield
>> (x, name))" is enough, no need to "if (isfield (x, name) == 1).
>>
>> It would be good practice to also have analyze75filename deal with the
>> filename part, just as it's doing for the other 2 functions. I'd guess
>> analyze75filename would take an extra argument ("write", "info" or
>> "read") for the part of checking if the file already exists. This
>> would also allow to make it return the right error message with:
>>
>> error ('analyze75%s: error message.', analyze75function)
>>
>> Carnë
>
> Those suggestions sound good, I can make those changes although I probably
> won't have time before the weekend.  If you're ready for the release of the
> image package then don't wait, as it won't affect functionality.

Not yet. There's some warnings and error messages for Mac users that
should be fixed before.

Carnë

Re: [OctDev] Analyze75

[adam a. <ada...@ho...>]

> From: car...@gm...
> Date: Tue, 2 Oct 2012 11:42:25 +0200
> Subject: Re: [OctDev] Analyze75
> To: ada...@ho...
> CC: oct...@li...
> 
> On 2 October 2012 11:11, adam aitkenhead <ada...@ho...> wrote:
> >> From: car...@gm...
> >> Date: Tue, 2 Oct 2012 09:33:17 +0200
> >> Subject: Re: [OctDev] Analyze75
> >> To: ada...@ho...
> >> CC: oct...@li...
> >>
> >> On 1 October 2012 22:30, adam aitkenhead <ada...@ho...>
> >> wrote:
> >> > I've just submitted analyze75write.m to SVN.
> >>
> >> Nice, thank you. The code looks nice. I have added the function to the
> >> INDEX and NEWS file, and to the see also of the other functions.
> >>
> >> Some tips, you don't need to check the value returned by isfield. If
> >> there is a field with that name, it will return true so "if (isfield
> >> (x, name))" is enough, no need to "if (isfield (x, name) == 1).
> >>
> >> It would be good practice to also have analyze75filename deal with the
> >> filename part, just as it's doing for the other 2 functions. I'd guess
> >> analyze75filename would take an extra argument ("write", "info" or
> >> "read") for the part of checking if the file already exists. This
> >> would also allow to make it return the right error message with:
> >>
> >> error ('analyze75%s: error message.', analyze75function)
> >>
> >> Carnë
> >
> > Those suggestions sound good, I can make those changes although I probably
> > won't have time before the weekend. If you're ready for the release of the
> > image package then don't wait, as it won't affect functionality.
> 
> Not yet. There's some warnings and error messages for Mac users that
> should be fixed before.
> 
> Carnë
 
Hi Carnë

I submitted a couple of changes to analyze75read and analyze75write last night to allow them to work properly for int16 and int32 images.  Still to do the other updates to analyze75write though.
 
Adam