Skip to content
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

SG 4.0.33 & PS 2.0.25 #594

Merged
merged 15 commits into from
May 21, 2021
Merged

Conversation

CarlosHorro
Copy link
Contributor

Metamorpheus available
Other bugfixes

@bernt-matthias
Copy link
Collaborator

Hey @CarlosHorro please add this here: #584

I also would prefer to have a version fixing compomics/peptide-shaker#445 as well. To me it seems that PS does not work at all due to this (or am I wrong?) and then there is not much use in updating SG since both tools are coupled, or?

@bernt-matthias
Copy link
Collaborator

A test using MM would also be cool.

# Conflicts:
#	tools/peptideshaker/macros_basic.xml
@CarlosHorro
Copy link
Contributor Author

Hey @CarlosHorro please add this here: #584

Yes, thank you, I forgot about it...

I also would prefer to have a version fixing compomics/peptide-shaker#445 as well. To me it seems that PS does not work at all due to this (or am I wrong?) and then there is not much use in updating SG since both tools are coupled, or?

Well, I might be forgetting something but as far as I know saying that "PS does not work at all" sounds a bit excessive?
SG and PS are not strictly coupled, it depends on the releases done to them both. They may work whist there are no big version differences between them.
I would like to work on that issue, but it is a separated one that will require quite an effort and I am not sure when I will be able to do it, so it is better to go step by step and fix this issue now (MM release) and that other issue in a specific release for it.

A test using MM would also be cool.

I might add it, but that would require to add mzml files to the repo as input to MM and they are usually too big to be included there...

Greetings,
Carlos

@CarlosHorro CarlosHorro changed the title SG 4.0.33 SG 4.0.33 & PS 2.0.25 May 12, 2021
@CarlosHorro
Copy link
Contributor Author

Well, now I see what you meant when you said that PS did not work at all. But paths.txt related stuff has not been touched for a long time and PS worked, so PS current failed tests should be related to another issue... I will try to dig into this as soon as possible, although now there are many holidays coming here in Norway :-)

Greetings,
Carlos

@bernt-matthias
Copy link
Collaborator

Well, now I see what you meant when you said that PS did not work at all

If this is a non critical error we could grep -v java.io.FileNotFoundException: /usr/local/share/peptide-shaker-2.0.25-0/resources/conf/paths.txt (Permission denied) from them stdout .. as a quick fix (or the exception could be catched in PS). Then the stdio regex would not complain.

@bernt-matthias
Copy link
Collaborator

I might add it, but that would require to add mzml files to the repo as input to MM and they are usually too big to be included there...

We might borrow one from here: #587

Is MM not working with mgf files? Then mgf input to peak_lists_files might lead to problems, or?

…n flow, as it happens when PS firstly tries to write the paths.txt file into a shared folder; so, the process should not automatically be considered as failed.
@CarlosHorro
Copy link
Contributor Author

I might add it, but that would require to add mzml files to the repo as input to MM and they are usually too big to be included there...

We might borrow one from here: #587

Is MM not working with mgf files? Then mgf input to peak_lists_files might lead to problems, or?

MetaMorpheus only admits mzml files, yes. I thought about adding a warning text message in the peak_lists_files area, and I am not sure if a validation may be added...

Copy link
Collaborator

@bernt-matthias bernt-matthias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one last question: is it correct that exec_dir is not used anymore for PS?

@bernt-matthias
Copy link
Collaborator

Can you respond to #553?

@CarlosHorro
Copy link
Contributor Author

Can you respond to #553?

Thank you for the reminder, I did not see her question on time.

@CarlosHorro
Copy link
Contributor Author

Just one last question: is it correct that exec_dir is not used anymore for PS?

Um... that is a good question, I think that using the temporary folder works fine, but now I am not sure about , ie, this searchgui.py code in bioconda :

def jvm_opts(argv):
...
elif arg.startswith('--exec_dir='):
            exec_dir = arg.split('=')[1].strip('"').strip("'")
            if not os.path.exists(exec_dir):
                shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None)
                src = real_dirname(sys.argv[0])
                os.remove(os.path.join(exec_dir, "resources/XTandem/linux/linux_64bit/tandem"))
                os.symlink(os.path.join(src, "resources/XTandem/linux/linux_64bit/tandem"),
                           os.path.join(exec_dir, "resources/XTandem/linux/linux_64bit/tandem"))

The piece of code related to xtandem is not being executed anymore, but it seems to work fine (I guess the path existed)...

@CarlosHorro
Copy link
Contributor Author

About the validator, do you think it is possible to do it @bernt-matthias ? I have done some research and I do not find anything related to a validator that checks the value of 2 different parameters (in this case, the search engines chosen and the file formats introduced) ?

@bernt-matthias
Copy link
Collaborator

bernt-matthias commented May 20, 2021

Um... that is a good question,

Wondering if this could "solve" the paths.txt exception problem since the copy should be writable.

But I would prefer if it works without exec_dir ..

@CarlosHorro
Copy link
Contributor Author

Um... that is a good question,

Wondering if this could "solve" the paths.txt exception problem since the copy should be writable.

But I would prefer if it works without exec_dir ..

You were right, with the "exec_dir" the paths.txt issue does not show up. What do I do? I am afraid that my time for keeping the tools will not be enough for solving that paths.txt related issue without the exec_dir soon at all...

@bernt-matthias
Copy link
Collaborator

What do I do?

I would suggest to try to go the grep way. Can you add a TODO note that exec_dir might not be needed anymore. Then we may remove it from the conda wrapper in the future once this has been tested for a while in the wild.

@bernt-matthias
Copy link
Collaborator

Really looking forward that my colleagues can test metamorpheus from SG (they are currently also trying #587).

@CarlosHorro
Copy link
Contributor Author

I will try to add the MetaMorpheus test today... do I add any comment about the source of the mzML & fasta files for the test, @bernt-matthias ?

@bernt-matthias
Copy link
Collaborator

do I add any comment about the source of the mzML & fasta files for the test,

yes. why not

@CarlosHorro
Copy link
Contributor Author

I think this should make it...

@bernt-matthias bernt-matthias merged commit 1cb321c into galaxyproteomics:master May 21, 2021
@bernt-matthias
Copy link
Collaborator

Thanks @CarlosHorro

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants