Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable reading of CRAM Header with no (explicit) reference #732

Closed

Conversation

yfarjoun
Copy link
Contributor

@yfarjoun yfarjoun commented Oct 27, 2016

Description

fixes #731 by allowing a CRAM file to be opened using a trivial referenceSource which has been added to the SamReaderFactory default settings.

  • added test cases to show that DictionaryExtractor works and also that reading the header of a cram file works.
  • fixed old tests that broke as they were getting a new (=unexpected) exception when opening a cram file for reading without a reference.
  • continued the march of java8 into the code
  • suppressed some intelliJ code inspection warnings and fixed a javadoc error.

Checklist

  • Code compiles correctly
  • New tests covering changes and new functionality
  • All tests passing
  • Extended the README / documentation, if necessary
  • Is not backward compatible (breaks binary or source compatibility)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 69.745% when pulling b74b332 on yf_make_get_header_possible_from_cram_with_no_reference into 58f4154 on master.

@@ -135,6 +137,7 @@ private SamReaderFactoryImpl(final EnumSet<Option> enabledOptions, final Validat
this.samRecordFactory = samRecordFactory;
this.validationStringency = validationStringency;
this.customReaderFactory = CustomReaderFactory.getInstance();
this.referenceSource = EMPTY_REFERENCE;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yfarjoun I think this will mask the fallback chain thats in place in the default reference source implementation (the code that uses environment variables to try the ENA download service or DEFAULT_FASTA settings - I don't think we have tests for those). Its currently established through the calls in SamReaderFactory to getDefaultCRAMReferenceSource. When we switched to using an interface based pattern for reference source for IGV, I intended to break those out into a separate implementations for each type (ENA ref service, DEFAULT_FASTA, user-supplied file) so we could establish which one to use once up front, but never did. For now they're still all mashed together in the one implementation.

@yfarjoun yfarjoun added the cram label Nov 22, 2016
@yfarjoun yfarjoun closed this Dec 12, 2016
@yfarjoun yfarjoun deleted the yf_make_get_header_possible_from_cram_with_no_reference branch October 15, 2017 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SAMSequenceDictionaryExtractor doesn't handle CRAMs
3 participants