Uauy-Lab / bioruby-polyploid-tools

Library and tools to deal with polyploid genomics
10 stars 11 forks source link

Polymarker changes for Grassroots usage #18

Closed billyfish closed 5 years ago

billyfish commented 5 years ago

Hi Ricky

I've done a small number of tweaks to your polymarker code to integrate it into our grassroots infrastructure (bear in mind my ruby knowledge is about zero :-) ! ) . They are allowing an existing output directory to be used, a bug fix to class Mask not being defined and a couple of tweaks to deal with when the chromosome isn't specified in the markers file.

Feel free to improve them and hope they're ok!

cheers

billy

homonecloco commented 5 years ago

Hi Billy, Thanks for this. Can you send the pull request with the same indentations? As it is now it is hard to read the diff. Thanks! Ricardo.

homonecloco commented 5 years ago

Also, On which cases you wouldn't have the chromosome specified? PolyMarker requires to have the chromosome specified, otherwise the results doesn't make sense. I think I have a different executable for those cases, but it is not as tested.

billyfish commented 5 years ago

Hi Ricky

Yup no problem, what indentations would you like? Spaces or tabs no problem. If spaces do you use 2? Basically let me know what you'd like and I'll do them :-)

cheers

billy

On Tue, 22 Jan 2019 at 17:30, Ricardo H. Ramírez-Gonzalez < notifications@github.com> wrote:

Hi Billy, Thanks for this. Can you send the pull request with the same indentations? As it is now it is hard to read the diff. Thanks! Ricardo.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/homonecloco/bioruby-polyploid-tools/pull/18#issuecomment-456488702, or mute the thread https://github.com/notifications/unsubscribe-auth/AHaM4VKRtYR5omDKYdAhqjwPHDeZWJurks5vF0rFgaJpZM4aNF-N .

homonecloco commented 5 years ago

Good question, Look at: https://github.com/homonecloco/bioruby-polyploid-tools/pull/18/files I don't know what is confusing the git diff, but it is clearly messing with something, as the lines are not aligned. I wonder if you added/removed indentation in some of the functions? R.

billyfish commented 5 years ago

Hi Ricky

We've set up polymarker against two more databases, the plan being to set it up against all of our blast databases once it's all been fully tested. For all of these, I'm not sure what chromosome information we've got. I ran a few test queries with and without the chromosome and it seemed to work ok, but you're the expert :-) So you'd recommend using it with the chromosomes, yeah? I'll see if we can get their details for each db.

cheers

billy

On Tue, 22 Jan 2019 at 17:33, Ricardo H. Ramírez-Gonzalez notifications@github.com wrote:

Also, On which cases you wouldn't have the chromosome specified? PolyMarker requires to have the chromosome specified, otherwise the results doesn't make sense. I think I have a different executable for those cases, but it is not as tested.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

billyfish commented 5 years ago

Ah!

I changed the

module Bio::PolyploidTools::Mask

to

module Bio::PolyploidTools class Mask

end

as ruby was complaining that Mask wasn't defined.

I indented all of the class code, I can unindent it easily enough if that is what you mean.

cheers

billy

On Tue, 22 Jan 2019 at 17:42, Ricardo H. Ramírez-Gonzalez < notifications@github.com> wrote:

Good question, Look at: https://github.com/homonecloco/bioruby-polyploid-tools/pull/18/files I don't know what is confusing the git diff, but it is clearly messing with something, as the lines are not aligned. I wonder if you added/removed indentation in some of the functions? R.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/homonecloco/bioruby-polyploid-tools/pull/18#issuecomment-456492760, or mute the thread https://github.com/notifications/unsubscribe-auth/AHaM4S4PZ3cMEoOF3BVAwo8BWpMGKxMZks5vF01ygaJpZM4aNF-N .

homonecloco commented 5 years ago

As for the Mask module/class:

Please return the indentation. This is an extremely critical file, so I want to be able to compare it line by line before merging it. It is not my best coding style, but "it works". Also, keep it as Module, you can add functions to a module directly. As Mask is never instantiated, it only has functions, not methods. I know in java everything is a Class, but in Ruby you can distinguish them (and Classes can include modules, but I'm not using that).

About the chromosome,

On your blast hit you should be able to get on which contig/chromosome the reference the best hit was, and based on that define the target chromosome. You also can change the method to parse the sequence names. On the current version of the website, I use the following function to validate that the chromosome is correct:

https://github.com/homonecloco/bioruby-polymarker/blob/master/app/helpers/reference_helper.rb#L26

Best, Ricardo.

homonecloco commented 5 years ago

Also, what are the new references you are adding? I can add them to the next version of the website without much trouble, I have that automated now.

billyfish commented 5 years ago

Hi Ricky

If I run it with the original Mask.rb where it's a module, I get:

7: from

/home/billy/Projects/bioruby-polyploid-tools/bin/polymarker.rb:11:in <main>' 6: from /usr/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in require' 5: from /usr/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in require' 4: from /home/billy/Projects/bioruby-polyploid-tools/lib/bioruby-polyploid-tools.rb:16:in <top (required)>' 3: from /home/billy/Projects/bioruby-polyploid-tools/lib/bioruby-polyploid-tools.rb:16:in each' 2: from /home/billy/Projects/bioruby-polyploid-tools/lib/bioruby-polyploid-tools.rb:18:in block in <top (required)>' 1: from /home/billy/Projects/bioruby-polyploid-tools/lib/bioruby-polyploid-tools.rb:18:in require_relative' /home/billy/Projects/bioruby-polyploid-tools/lib/bio/PolyploidTools/Mask.rb:11:in <top (required)>': uninitialized constant Bio::PolyploidTools (NameError)

The same as the issue at https://github.com/homonecloco/bioruby-polyploid-tools/issues/17

Like I said, I'm definitely not a Ruby expert :-) so I just guessed about the class thing.

For the blast hit, I can get the scaffold as we have the samtools indexes for them, I'll take a look at your helper code :-)

cheers

billy

On Tue, 22 Jan 2019 at 17:55, Ricardo H. Ramírez-Gonzalez < notifications@github.com> wrote:

As for the Mask module/class:

Please return the indentation. This is an extremely critical file, so I want to be able to compare it line by line before merging it. It is not my best coding style, but "it works". Also, keep it as Module, you can add functions to a module directly. As Mask is never instantiated, it only has functions, not methods. I know in java everything is a Class, but in Ruby you can distinguish them (and Classes can include modules, but I'm not using that).

About the chromosome,

On your blast hit you should be able to get on which contig/chromosome the reference the best hit was, and based on that define the target chromosome. You also can change the method to parse the sequence names. On the current version of the website, I use the following function to validate that the chromosome is correct:

https://github.com/homonecloco/bioruby-polymarker/blob/master/app/helpers/reference_helper.rb#L26

Best, Ricardo.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/homonecloco/bioruby-polyploid-tools/pull/18#issuecomment-456497604, or mute the thread https://github.com/notifications/unsubscribe-auth/AHaM4SNFFN9t9EDIdVIgu2oQdgiYCx5Oks5vF1CpgaJpZM4aNF-N .

billyfish commented 5 years ago

Hi Ricky

As I feared, most of our references don't have the chromosome information :-( Polymarker still runs but if you're saying you have another executable that is more appropriate, that'd be great.

cheers

billy

On Tue, 22 Jan 2019 at 18:03, Simon Tyrrell tyrrell.simon@gmail.com wrote:

Hi Ricky

If I run it with the original Mask.rb where it's a module, I get:

7: from

/home/billy/Projects/bioruby-polyploid-tools/bin/polymarker.rb:11:in <main>' 6: from /usr/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in require' 5: from /usr/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in require' 4: from /home/billy/Projects/bioruby-polyploid-tools/lib/bioruby-polyploid-tools.rb:16:in <top (required)>' 3: from /home/billy/Projects/bioruby-polyploid-tools/lib/bioruby-polyploid-tools.rb:16:in each' 2: from /home/billy/Projects/bioruby-polyploid-tools/lib/bioruby-polyploid-tools.rb:18:in block in <top (required)>' 1: from /home/billy/Projects/bioruby-polyploid-tools/lib/bioruby-polyploid-tools.rb:18:in require_relative' /home/billy/Projects/bioruby-polyploid-tools/lib/bio/PolyploidTools/Mask.rb:11:in <top (required)>': uninitialized constant Bio::PolyploidTools (NameError)

The same as the issue at https://github.com/homonecloco/bioruby-polyploid-tools/issues/17

Like I said, I'm definitely not a Ruby expert :-) so I just guessed about the class thing.

For the blast hit, I can get the scaffold as we have the samtools indexes for them, I'll take a look at your helper code :-)

cheers

billy

On Tue, 22 Jan 2019 at 17:55, Ricardo H. Ramírez-Gonzalez < notifications@github.com> wrote:

As for the Mask module/class:

Please return the indentation. This is an extremely critical file, so I want to be able to compare it line by line before merging it. It is not my best coding style, but "it works". Also, keep it as Module, you can add functions to a module directly. As Mask is never instantiated, it only has functions, not methods. I know in java everything is a Class, but in Ruby you can distinguish them (and Classes can include modules, but I'm not using that).

About the chromosome,

On your blast hit you should be able to get on which contig/chromosome the reference the best hit was, and based on that define the target chromosome. You also can change the method to parse the sequence names. On the current version of the website, I use the following function to validate that the chromosome is correct:

https://github.com/homonecloco/bioruby-polymarker/blob/master/app/helpers/reference_helper.rb#L26

Best, Ricardo.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/homonecloco/bioruby-polyploid-tools/pull/18#issuecomment-456497604, or mute the thread https://github.com/notifications/unsubscribe-auth/AHaM4SNFFN9t9EDIdVIgu2oQdgiYCx5Oks5vF1CpgaJpZM4aNF-N .

homonecloco commented 5 years ago

Can you tell me the command you are using to run polymarker? There is an option that you can use to just use the scaffold, it would be something like this:

polymarker.rb --arm_selection scaffold ....

Then on the second column of the input you should put the scaffold name. I'm going over to EI tomorrow, I'll see if I can drop in so we can chat this in person.

billyfish commented 5 years ago

Hi Ricky

I'm effectively running

polymarker.rb --contigs ~/databases/Triticum_aestivum.IWGSC2.25.dna.toplevel.fa --marker_list ~/working_directory/polymarker/test/markers_list --output ~/working_directory/polymarker/test --aligner blast --primer_3_preferences ~/working_directory/polymarker/test/primer3.prefs

and can easily add/change parameters

Chatting about it in person would be probably be a lot easier! :-)

cheers

billy

On Wed, 23 Jan 2019 at 15:35, Ricardo H. Ramírez-Gonzalez notifications@github.com wrote:

Can you tell me the command you are using to run polymarker? There is an option that you can use to just use the scaffold, it would be something like this:

polymarker.rb --arm_selection scaffold ....

Then on the second column of the input you should put the scaffold name. I'm going over to EI tomorrow, I'll see if I can drop in so we can chat this in person.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.