Adding collision energy in precursor. #12
Conversation
| @@ -29,6 +29,7 @@ impl PrecursorReader { | |||
| let precursor_table: PrecursorTable = | |||
| PrecursorTable::from_sql(&tdf_reader.tdf_sql_reader); | |||
| let retention_times: Vec<f64> = tdf_reader.frame_table.rt.clone(); | |||
| let collision_energy = &pasef_frames.collision_energy_by_precursor; | |||
There was a problem hiding this comment.
Make this plural collision_energies to improve readability
| @@ -28,6 +29,7 @@ impl ReadableFromSql for PasefFrameMsMsTable { | |||
| .read_column_from_table("CollisionEnergy", table_name), | |||
| precursor: sql_reader | |||
| .read_column_from_table("Precursor", table_name), | |||
| collision_energy_by_precursor: sql_reader.get_data_from_sql(&"select CollisionEnergy from PasefFrameMsMsInfo group by Precursor".to_string()), | |||
There was a problem hiding this comment.
I think this conflicts with the setup of this class. It should just use the read_from_table structure to ensure the length of the array and the order is consistent with the rest of the table. Parsing it for a specific use case should happen at the appropriate spot, not here.
| @@ -29,6 +29,7 @@ impl PrecursorReader { | |||
| let precursor_table: PrecursorTable = | |||
| PrecursorTable::from_sql(&tdf_reader.tdf_sql_reader); | |||
| let retention_times: Vec<f64> = tdf_reader.frame_table.rt.clone(); | |||
| let collision_energy = &pasef_frames.collision_energy_by_precursor; | |||
There was a problem hiding this comment.
This is probably yhe place to do group by precursor you currently do in the table loading.
| let column_names: Vec<String> = | ||
| self.get_table_columns(table_name).unwrap(); | ||
| let order_by: String = column_names.join(", "); | ||
| let query: String = format!( | ||
| "SELECT {} FROM {} ORDER BY {}", | ||
| column_name, table_name, order_by | ||
| ); | ||
|
|
There was a problem hiding this comment.
I like that you adhere to the boy scout rule here and leave things better than you found them. Good addition to improve code readability, thanks!
| fn read_column_from_table< | ||
| T: rusqlite::types::FromSql + std::default::Default, | ||
| >( | ||
| fn read_column_from_table<T: rusqlite::types::FromSql + Default>( |
There was a problem hiding this comment.
Very minimal change, check if if it formatted right by running cargo fmt. Normally thse kind sof changes are done automaticaly. (I evne set up my editor (VSCode with rust-analyzer extension) to run cargo fmt automatically on saving)
|
Test seem to be failing, we need to add collision enegries to them, or even add a default to simplify things... |
Added default value in the Precursor object. test cases are working now. |
Hi @swillems ,
I have made the necessary modifications to the precursor object by adjusting the collision energy. Kindly take a moment to review and provide any feedback or suggestions for further changes.
Thanks,
-Vijay