Skip to content

Conversation

@Colelyman
Copy link
Contributor

@kclem is there a particular reason why this alignment matrix can't be customized?

@kclem
Copy link
Member

kclem commented Nov 17, 2023

The problem is that _ROOT is dynamic at runtime depending on where the package is installed, and the EDNAFULL matrix file is placed there during installation. This works for the arg default 'EDNAFULL', but if we prepend args.needleman_wunsch_aln_matrix_loc by ROOT, that won't produce a valid path for users who specify their own file.

Some other options would be:

  1. have args.needleman_wunsch_aln_matrix_loc default to None, and if at runtime the arg is None, we look for the EDNAFULL in _ROOT. However, this may be confusing to users ("The default value is None?!?") although we could potentially compassionately deceive users by telling them the default is ENDAFULL and have the default really be None
  2. get rid of the EDNAFULL matrix file and just recreate it in memory

@Colelyman
Copy link
Contributor Author

Ah, I see what it is like that now; I figured there was a good reason :).

  1. I like this option because it keeps the file separate which makes it easier to modify. Also, if a user wants a custom matrix, but doesn't want to supply --needleman_wunsch_aln_matrix with every run, they can just modify the EDNAFULL file, et voila. (They would need to update it with each install, so it wouldn't be very practical... but I think it is more transparent?)
  2. Not a bad idea either, the only drawback I see is that it makes it slightly more difficult for a user to know how to format a custom matrix file.

Any opposition to me implementing the first option?

@Colelyman
Copy link
Contributor Author

On second thought, isn't this line only executed at runtime (it is inside the main function)? So we should be able to use args.needleman_wunsch_aln_matrix_loc, unless I'm missing something.

@Colelyman Colelyman marked this pull request as draft April 10, 2024 15:50
Colelyman and others added 2 commits December 23, 2025 12:00
* Mckay/be plot improvements (#136)

* trying to get the figure to fit nicely, increased
element size to 100

* custom figsize to display without cutting off

increased figsize in report template

* Allow messages to be served in CLI reports (#134) (pinellolab#583)

* Fix deletion at second position (#131)

* Fix bug when there is a deletion starting at the second position

This bug only happens when a deletion starts are the second position, before the
fix, it would report that the deletion started at the first position. It is
fixed now, so deletions at the second position are reported correctly.

* Update CRISPRessoCOREResources.c due to change in .pyx

* Add tests for find_indels_substitutions for deletions at the end

* Fix 1bp deletions at the end, and off by one error

This ensures that when a deletion occurs at the end of a read, the entire
deletion is accounted for.

* Update CRISPRessoCOREResources.c to reflect fixes for deletions at the end of alignments

* Add extra asserts to deletion checks

* Point to new test branch

* Reafctor deletion_coordinates to go past the end of the string for deletions at
the end of the sequence

* Point tests to master

* Allow messages to be served in CLI reports

* Point to cole/messages test branch

* Point tests back to master

* point to tests branch

* typo

* testing github actions

* remove test

* point tests to master

---------

Co-authored-by: Cole Lyman <[email protected]>

* Update inferred QWC tests to reflect correct intended behavior

* Fix inferring QWC to match intended behavior

* Add more test cases and fix bug discovered in single bp QWC

* Add even more test cases testing indels outside the QWC

* Point tests to cole/fix-qwc-deletion

* update plotly.js (#138)

* Change order of amplicon inference alignment so that 1st amplicon is the reference

This makes a difference because it changes the values of `s1inds`, and therefore
the value of the inferred quantification window coordinates.

* Point integration tests back to master

* Update CHANGELOG.md

---------

Co-authored-by: mbowcut2 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants