Re: [sr #3044] Load spins from SPARKY list -- August 09, 2013 - 13:30

Hi,

I like the idea of reusing the code!  I've been thinking and maybe
there is an even better solution, one that is much more flexible.  I
am trying to shift as much code as possible into the relax library.
Any code which does not relate to the specific analyses or does not
touch the relax data store should be shifted into the relax library.
The whole concept of reading peak list data fits into this.  So I'm
wondering about creating a new module called lib.spectrum and creating
a function called read_peak_list().  This function would then be used
to read all types of data from a peak list, independent of the format
of that list.  With this, a lot of code from
pipe_control.spectrum.read() would be shifted into this new function.
So instead of using pipe_control.spectrum.read(), you call
lib.spectrum.read_peak_list() to do all the work.  The generic peak
list code in pipe_control.spectrum would also shift into lib.spectrum.

One reason this would be useful is because I will likely implement the
reading of chemical shifts from these peaks lists for the
off-resonance R1rho support in the relax_disp branch.  This would
likely go into a new module called pipe_control.chemical_shift.  This
code could then call the lib.spectrum.read_peak_list().  The
pipe_control.spectrum.read() function could be renamed to
pipe_control.spectrum.read_intensities() and call this function too.
And the spectrum.read_sequence user function backend is called
pipe_control.spectrum.read_sequence() which also calls this function.
The spin ID can be then built by each pipe_control function, but only
if required.  For example, when generating the sequence data as spin
ID is not needed.  It's only used to return a pre-existing spin.  But
the spin ID generation function is dependent on the relax data store,
so it cannot be called from the relax library.

For the lib.spectrum.read_peak_list() function, we could have
arguments to select if the chemical shifts should be returned, the
peak intensity, etc.  One argument for each type of data present in a
peak list, all defaulting to None.  This function can return the
molecule name, residue name, residue number, spin name, and spin
number (defaulting to None when no data is present) as the first
elements, followed by what ever other data is asked for.  What do you
think?

Regards,

Edward

On 9 August 2013 12:11, Troels Emtekær Linnet <tlinnet@xxxxxxxxx> wrote:
> Hi Edward.
>
> Thanks for your suggestions.
>
> I was carried away, to try to make some GUI changes.
> Just to see, how it worked.
>
> So, I have thought a little about the design since then.
>
> I was thinking to take advantage of the: .\pipe_control\spectrum.py read()
> function.
>
> I already provides a spin_id in its functions.
>
> And so the functions could be modified to create spins instead of reading
> the
> intensities.
>
> I think the easiest would be to provide another keyword to the read()
> function.
> Something like: return_model = False
>
> If then read() is called with return_model = True
> the checks and reading of intensities are skipped, but the
> spin_ids are returned.
>
> What do you think of this?
>
>
>
>
> Troels Emtekær Linnet
>
>
> 2013/8/3 Edward d'Auvergne <edward@xxxxxxxxxxxxx>
> > Hi,
> >
> > It might be better to discuss the design a bit before.  I've looked at
> > the patches and although half are ok, I think it is a little too
> > specific for relax.  By that I mean that this idea can be generalised,
> > as is the design goal for most of relax.  So instead of being Sparky
> > specific, everything should be generalised to be a 'peak list'.  That
> > way support can be added for NMRPipe series Tab, NMRView, XEasy, Cara,
> > CCPN, etc. later much more easily - i.e. only the user function front
> > and backends need to be modified.  That is how the
> > spectrum.read_intensities user function works.
> >
> > For the user function name itself, the best might be
> > spectrum.read_sequence.  Though an alternative place might be under
> > sequence.peak_list.  The first one might be more logical from the code
> > perspective as the backend will be in pipe_control.spectrum.  But I'm
> > not sure for a user which would be easier to find.
> >
> > Anyway, your patches won't take much to modify.  However I think
> > patches 0001 to 0005 should be one commit.  0006 would need a little
> > work.  I think that *.* should be the default for the file arguments,
> > and then the user can select *.ser, *.xpk, *.list and *.text (for
> > NMRPipe, NMRView, Sparky and XEasy) to be more specific when selecting
> > the file.  I think that the molecule should also be left unnamed if
> > the user does not supply a molecule name.  They can always use
> > molecule.name later.  It also needs a description and title along the
> > lines of the sequence.read user function, as its front end design and
> > its aim are similar.  So the title might be "Read the molecule,
> > residue, and spin sequence from a peak list.".  And the short title
> > something like "Sequence reading from peak lists.".  What do you
> > think?
> >
> > Patch 0007* has the trailing whitespace removal problem, so it seems
> > like you haven't fully eliminated that issue.  In this case that
> > whitespace should be removed, but it should be a separate trivial
> > patch.  The code is also in the wrong place, the backend should be in
> > pipe_control.spectrum, and it should be modelled on the read()
> > function (maybe called read_sequence()).  The 0008* patch is also
> > quite far off target.  This is why it is better to discuss a design
> > idea on the mailing list before implementing it.  With discussions,
> > the best solution can be found from the start.
> >
> > Cheers,
> >
> > Edward
> >
> >
> >
> > On 3 August 2013 08:56, Troels E. Linnet
> > <NO-REPLY.INVALID-ADDRESS@xxxxxxx> wrote:
> > > Follow-up Comment #2, sr #3044 (project relax):
> > >
> > > Initial tries for a SPARKY/NMRPipe seriesTab GUI read of spins.
> > >
> > > If these patches are ok, it would be time to establish system tests, and
> > > I
> > > hope for a little guide in which files these should be written.
> > >
> > > (file #18632)
> > >     _______________________________________________________
> > >
> > > Additional Item Attachment:
> > >
> > > File name: sparky1.patch.tar.gz           Size:4 KB
> > >
> > >
> > >     _______________________________________________________
> > >
> > > Reply to this item at:
> > >
> > >   <http://gna.org/support/?3044>
> > >
> > > _______________________________________________
> > >   Message sent via/by Gna!
> > >   http://gna.org/
> > >
> > >
> > > _______________________________________________
> > > relax (http://www.nmr-relax.com)
> > >
> > > This is the relax-devel mailing list
> > > relax-devel@xxxxxxx
> > >
> > > To unsubscribe from this list, get a password
> > > reminder, or change your subscription options,
> > > visit the list information page at
> > > https://mail.gna.org/listinfo/relax-devel