Fix plots and improve plot error handling (#431)

* Sam/try plots (#71) * Fix batch mode pandas warning. (#70) * refactor to call method on DataFrame, rather than Series. Removes warning. * Fix pandas future warning in CRISPRessoWGS --------- Co-authored-by: Cole Lyman <cole@colelyman.com> * Functional * Cole/fix status file name (#69) * Update config file logging messages This removes printing the exception (which is essentially a duplicate), and adds a condition if no config file was provided. Also changes `json` to `config` so that it is more clear. * Fix divide by zero when no amplicons are present in Batch mode * Don't append file_prefix to status file name * Place status files in output directories * Update tests branch for file_prefix addition * Load D3 and plotly figures with pro with multiple amplicons * Update batch * Fix bug in CRISPRessoCompare with pointing to report datas with file_prefix Before this fix, when using a file_prefix the second run that was compared would not be displayed as a data in the first figure of the report. * Import CRISPRessoPro instead of importing the version When installed via conda, the version is not available * Remove `get_amplicon_output` unused function from CRISPRessoCompare Also remove unused argparse import * Implement `get_matching_allele_files` in CRISPRessoCompare and accompanying unit tests * Allow for matching of multiple guides in the same amplicon * Fix pandas FutureWarning * Change test branch back to master --------- Co-authored-by: Sam <snic9004@gmail.com> * Try catch all futures * Fix test fail plots * Point test to try-plots * Fix d3 not showing and plotly mixing with matplotlib * Use logger for warnings and debug statements * Point tests back at master --------- Co-authored-by: mbowcut2 <55161542+mbowcut2@users.noreply.github.com> Co-authored-by: Cole Lyman <cole@colelyman.com> * Sam/fix plots (#72) * Fix batch mode pandas warning. (#70) * refactor to call method on DataFrame, rather than Series. Removes warning. * Fix pandas future warning in CRISPRessoWGS --------- Co-authored-by: Cole Lyman <cole@colelyman.com> * Functional * Cole/fix status file name (#69) * Update config file logging messages This removes printing the exception (which is essentially a duplicate), and adds a condition if no config file was provided. Also changes `json` to `config` so that it is more clear. * Fix divide by zero when no amplicons are present in Batch mode * Don't append file_prefix to status file name * Place status files in output directories * Update tests branch for file_prefix addition * Load D3 and plotly figures with pro with multiple amplicons * Update batch * Fix bug in CRISPRessoCompare with pointing to report datas with file_prefix Before this fix, when using a file_prefix the second run that was compared would not be displayed as a data in the first figure of the report. * Import CRISPRessoPro instead of importing the version When installed via conda, the version is not available * Remove `get_amplicon_output` unused function from CRISPRessoCompare Also remove unused argparse import * Implement `get_matching_allele_files` in CRISPRessoCompare and accompanying unit tests * Allow for matching of multiple guides in the same amplicon * Fix pandas FutureWarning * Change test branch back to master --------- Co-authored-by: Sam <snic9004@gmail.com> * Try catch all futures * Fix test fail plots * Fix d3 not showing and plotly mixing with matplotlib --------- Co-authored-by: mbowcut2 <55161542+mbowcut2@users.noreply.github.com> Co-authored-by: Cole Lyman <cole@colelyman.com> * Remove token from integration tests file --------- Co-authored-by: Samuel Nichols <Snic9004@gmail.com> Co-authored-by: mbowcut2 <55161542+mbowcut2@users.noreply.github.com>
pinellolab · May 9, 2024 · 1c50427 · 1c50427
1 parent acb2ea8
commit 1c50427
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 20 deletions.
diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml
@@ -45,7 +45,6 @@ jobs:
       uses: actions/checkout@v3
       with:
         repository: edilytics/CRISPResso2_tests
-        token: ${{ secrets.ACCESS_CRISPRESSO2_TESTS }}
         # ref: '<BRANCH-NAME>' # Use this to specify a branch other than master
 
     - name: Run Basic

diff --git a/CRISPResso2/CRISPRessoAggregateCORE.py b/CRISPResso2/CRISPRessoAggregateCORE.py
@@ -869,7 +869,12 @@ def main():
                 debug('Plot pool results:')
                 for future in process_futures:
                     debug('future: ' + str(future))
-            future_results = [f.result() for f in process_futures] #required to raise exceptions thrown from within future
+            for future in process_futures:
+                try:
+                    future.result()
+                except Exception as e:
+                    logger.warning('Error in plot pool: %s' % e)
+                    logger.debug(traceback.format_exc())
             process_pool.shutdown()
 
         info('Analysis Complete!', {'percent_complete': 100})

diff --git a/CRISPResso2/CRISPRessoBatchCORE.py b/CRISPResso2/CRISPRessoBatchCORE.py
@@ -904,7 +904,12 @@ def main():
                 debug('CRISPResso batch results:')
                 for future in process_futures:
                     debug('future: ' + str(future))
-            future_results = [f.result() for f in process_futures] #required to raise exceptions thrown from within future
+            for future in process_futures:
+                try:
+                    future.result()
+                except Exception as e:
+                    logger.warning('Error in plot pool: %s' % e)
+                    logger.debug(traceback.format_exc())
             process_pool.shutdown()
 
         if not args.suppress_report:

diff --git a/CRISPResso2/CRISPRessoCORE.py b/CRISPResso2/CRISPRessoCORE.py
@@ -3776,11 +3776,11 @@ def count_alternate_alleles(sub_base_vectors, ref_name, ref_sequence, ref_total_
                     mod_df_for_plot.insert(0, 'Batch', ref_name)
 
                     plot_root = _jp('2a.'+ ref_plot_name + 'Nucleotide_percentage_quilt')
-                    pro_output_name = f'plot_{os.path.basename(plot_root)}.json'
+                    pro_output_name = os.path.join(OUTPUT_DIRECTORY, f'plot_{os.path.basename(plot_root)}.json')
                     plot_2a_input = {
                         'nuc_pct_df': nuc_df_for_plot,
                         'mod_pct_df': mod_df_for_plot,
-                        'fig_filename_root': f'{_jp(pro_output_name)}' if not args.use_matplotlib and C2PRO_INSTALLED else plot_root,
+                        'fig_filename_root': pro_output_name if not args.use_matplotlib and C2PRO_INSTALLED else plot_root,
                         'save_also_png': save_png,
                         'sgRNA_intervals': sgRNA_intervals,
                         'sgRNA_names': sgRNA_names,
@@ -3824,11 +3824,11 @@ def count_alternate_alleles(sub_base_vectors, ref_name, ref_sequence, ref_total_
                         for x in include_idxs_list:
                             new_include_idx += [x - new_sel_cols_start]
                         plot_root = _jp('2b.'+ ref_plot_name + 'Nucleotide_percentage_quilt_around_' + sgRNA_label)
-                        pro_output_name = f'plot_{os.path.basename(plot_root)}.json'
+                        pro_output_name = os.path.join(OUTPUT_DIRECTORY, f'plot_{os.path.basename(plot_root)}.json')
                         plot_2b_input = {
                             'nuc_pct_df': nuc_df_for_plot.iloc[:, sel_cols],
                             'mod_pct_df': mod_df_for_plot.iloc[:, sel_cols],
-                            'fig_filename_root': f'{_jp(pro_output_name)}' if not args.use_matplotlib and C2PRO_INSTALLED else plot_root,
+                            'fig_filename_root': pro_output_name if not args.use_matplotlib and C2PRO_INSTALLED else plot_root,
                             'save_also_png': save_png,
                             'sgRNA_intervals': new_sgRNA_intervals,
                             'sgRNA_names': sgRNA_names,
@@ -4893,7 +4893,12 @@ def get_scaffold_len(row, scaffold_start_loc, scaffold_seq):
                 debug('Plot pool results:')
                 for future in process_futures:
                     debug('future: ' + str(future))
-            future_results = [f.result() for f in process_futures] #required to raise exceptions thrown from within future
+            for future in process_futures:
+                try:
+                    future.result()
+                except Exception as e:
+                    logger.warning('Error in plot pool: %s' % e)
+                    logger.debug(traceback.format_exc())
             process_pool.shutdown()
 
         info('Done!')

diff --git a/CRISPResso2/CRISPRessoMultiProcessing.py b/CRISPResso2/CRISPRessoMultiProcessing.py
@@ -14,6 +14,7 @@
 from inspect import getmodule, stack
 import numpy as np
 import pandas as pd
+import traceback
 
 def get_max_processes():
     return mp.cpu_count()
@@ -271,7 +272,13 @@ def run_plot(plot_func, plot_args, num_processes, process_futures, process_pool)
     -------
     None
     """
-    if num_processes > 1:
-        process_futures[process_pool.submit(plot_func, **plot_args)] = (plot_func, plot_args)
-    else:
-        plot_func(**plot_args)
+    logger = logging.getLogger(getmodule(stack()[1][0]).__name__)
+    try:
+        if num_processes > 1:
+            process_futures[process_pool.submit(plot_func, **plot_args)] = (plot_func, plot_args)
+        else:
+            plot_func(**plot_args)
+    except Exception as e:
+        logger.warn(f"Plot error {e}, skipping plot \n")
+        logger.debug(traceback.format_exc())
+
diff --git a/CRISPResso2/CRISPRessoReports/CRISPRessoReport.py b/CRISPResso2/CRISPRessoReports/CRISPRessoReport.py
@@ -588,7 +588,6 @@ def fill_default(dictionary, key, default_type=list):
             'datas': [],
             'htmls': [],
         }
-
     for html in sub_html_files:
         sub_html_files[html] = crispresso_data_path + sub_html_files[html]
     with open(crispresso_multi_report_file, 'w', encoding="utf-8") as outfile:

diff --git a/CRISPResso2/CRISPRessoReports/templates/shared/partials/fig_reports.html b/CRISPResso2/CRISPRessoReports/templates/shared/partials/fig_reports.html
@@ -13,7 +13,7 @@
     {%- if amplicon_name is defined -%}
         {%- if 'htmls' in report_data['figures'] and fig_name in report_data['figures']['htmls'][amplicon_name] -%}
             {{report_data['figures']['htmls'][amplicon_name][fig_name]|safe}}
-        {%- else -%}
+        {%- elif fig_name in report_data['figures']['locs'][amplicon_name] -%}
             <a href="{{report_data['crispresso_data_path']}}{{report_data['figures']['locs'][amplicon_name][fig_name]}}.pdf"><img src="{{report_data['crispresso_data_path']}}{{report_data['figures']['locs'][amplicon_name][fig_name]}}.png" width='{{width}}'></a>
         {% endif -%}
         <label class="labelpadding">{{report_data['figures']['captions'][amplicon_name][fig_name]}}</label>
@@ -23,12 +23,14 @@
     {%- else %}
         {%- if 'htmls' in report_data['figures'] and fig_name in report_data['figures']['htmls'] -%}
             {{report_data['figures']['htmls'][fig_name]|safe}}
-        {%- else -%}
+        {%- elif fig_name in report_data['figures']['locs'] -%}
             <a href="{{report_data['crispresso_data_path']}}{{report_data['figures']['locs'][fig_name]}}.pdf"><img src="{{report_data['crispresso_data_path']}}{{report_data['figures']['locs'][fig_name]}}.png" width='{{width}}'></a>
         {% endif -%}
-        <label class="labelpadding">{{report_data['figures']['captions'][fig_name]}}</label>
-        {%- for (data_label,data_path) in report_data['figures']['datas'][fig_name] %}
-            <p class="m-0"><small>Data: <a href="{{report_data['crispresso_data_path']}}{{data_path}}">{{data_label}}</a></small></p>
-        {%- endfor -%}
+        {% if fig_name in report_data['figures']['captions'] and fig_name in report_data['figures']['datas'] %}
+            <label class="labelpadding">{{report_data['figures']['captions'][fig_name]}}</label>
+            {%- for (data_label,data_path) in report_data['figures']['datas'][fig_name] %}
+                <p class="m-0"><small>Data: <a href="{{report_data['crispresso_data_path']}}{{data_path}}">{{data_label}}</a></small></p>
+            {%- endfor -%}
+        {%- endif %}
     {%- endif %}
-</div>
+</div>