New tool addition: Samestr2 #7304
Conversation
|
Hi @SaimMomin12 could I get an initial review of the tools? I was able to solve the failing tests in the PR. I still get a lint error that I do not get in the terminal so I'm unsure on how to fix it. |
SaimMomin12
left a comment
There was a problem hiding this comment.
Hi @xens25
Following up. I’ve added a few preliminary inline comments. We’ll need to build a data manager for samestr_db, which can then be reused downstream across the other tools.
|
Hi @SaimMomin12 , I have now added the samestr_db as a data manager. Let me know in case there's further things to modify. Thanks in advanced. |
@xens25 Thanks, could you create a seperate PR for the DM? |
|
Hi @wm75 , let me know in case more changes need to be done. Thanks in advance! |
|
@xens25 I fixed up everything including another refactoring of the filter wrapper which is now back at consuming single datasets. Remaining tasks for you would be to:
|
|
Hi @wm75 , I added the samples-select, marker-remove and marker-keep tests in Samestr Filter. I wasn't exactly sure which things needed to be improved in the help sections and help tags, so I just tried to change some to use similar wording that you used in the other tools. Let me know in case some still need to be improved. As for planemo serve, in Samestr convert I get for (SameStr Convert on : per-clade SNV profiles) the following: a list with 0 datasets . I'm not sure why that is since the test is the same that passed in planemo test. I think it might be related to the fact that npz files are stored not directly in out_convert but inside a subdirectory with the name of the file, in this case out_convert/phage_padded, but I'm not sure if that's the reason. The rest of the tools worked as intended. |
Are you sure you configured things exactly like in that test? In particular, have you set "min_vcov" to 1? When I tried this last it was working. |
You are right! I completely missed that, I tested it again, Samestr Convert works fine! |
|
Cool! Then I don't think I have anything more to add here. @SaimMomin12 ready from my side! |
Many thanks @wm75 for the thorough and detailed review 🙏 |
|
Deployment status: failure https://github.com/galaxyproject/tools-iuc/actions/runs/24440679374 |
@xens25 could you check on this and create a corresponding PR? |
|
@xens25 looks like we simply forgot to put the |
FOR CONTRIBUTOR: