Assembly annotation #1736
Merged
Assembly annotation #1736
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Code Climate has analyzed commit 63a3ae0 and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
cdebourcy
reviewed
Nov 14, 2018
app/models/pipeline_run.rb
Outdated
| taxid_list = [] | ||
| contig2taxid.values.each { |entry| taxid_list += entry.values } | ||
| taxon_lineage_map = {} | ||
| TaxonLineage.where(taxid: taxid_list.uniq).each { |t| taxon_lineage_map[t.taxid.to_i] = t.to_a } |
There was a problem hiding this comment.
do you need to select only the most recent / the most appropriate?
There was a problem hiding this comment.
great point. i can make it oder by id so the latest one will always be there.
cdebourcy
reviewed
Nov 14, 2018
app/models/taxon_lineage.rb
Outdated
| @@ -159,6 +159,21 @@ def self.fill_lineage_details(tax_map, pipeline_run_id) | |||
| tax_map | |||
| end | |||
|
|
|||
| def to_a | |||
| [species_taxid, genus_taxid, family_taxid, order_taxid, class_taxid, phylum_taxid, | |||
There was a problem hiding this comment.
Could the following help with maintainability?
def to_a
TaxonLineage.names_a.map { |col| self[col] }
end
and
def self.null_array
TaxonLineage.column_defaults.values_at(*TaxonLineage.names_a)
end
That way, the list of columns is only hardcoded in names_a. I think it would be preferable
cdebourcy
approved these changes
Nov 15, 2018
app/models/pipeline_run.rb
Outdated
| @@ -365,9 +419,15 @@ def db_load_contig_counts | |||
| tax_level: tax_entry['tax_level'], | |||
| contig_name: contig_name, | |||
| count: count } | |||
| if tax_entry['tax_level'].to_i == 1 # species | |||
There was a problem hiding this comment.
TaxonCount:: TAX_LEVEL_SPECIES?
app/models/pipeline_run.rb
Outdated
| end | ||
|
|
||
| def contigs_fasta_s3_path | ||
| return "#{postprocess_output_s3_path}/#{ASSEMBLED_CONTIGS_NAME}" if pipeline_version && pipeline_version.to_f >= 3.1 |
There was a problem hiding this comment.
should we define ASSEMBLY_PIPELINE_VERSION = '3.1'.freeze?
kislyuk
pushed a commit
that referenced
this pull request
Mar 15, 2022
Validate names after group lanes for BaseSpace data
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Type of change
Test and Reproduce
Outline the steps to test or reproduce the PR here.
Impacted Areas in Application
List general components of the application that this PR will affect:
Which pages?
Specific components
Testing Script:
###Login Page
###Single Upload
###Batch Upload
###All Project Page
###Sample Details Page
###Report Page
Checklist: