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
Re: [sr #3044] Load spins from SPARKY list