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

screed.Record(..., quality=None) should not set 'quality' #1468

Closed
ctb opened this issue Oct 1, 2016 · 8 comments
Closed

screed.Record(..., quality=None) should not set 'quality' #1468

ctb opened this issue Oct 1, 2016 · 8 comments

Comments

@ctb
Copy link
Member

ctb commented Oct 1, 2016

Right now, you can't pass 'None' in with 'record' in order to create a Record without a quality entry. Should be able to.

@ctb ctb added this to the screed 1.0 milestone Oct 6, 2016
@luizirber
Copy link
Member

When you create a new Record like this:

>>> screed.Record(name="test", sequence="ACGT", quality=None)

the result should be:

  1. {'name': 'test', 'sequence': 'ACGT', 'quality': None}
  2. {'name': 'test', 'sequence': 'ACGT', 'quality': ""}
  3. {'name': 'test', 'sequence': 'ACGT'}

1 is the current result, 2 is consistent with khmer.ReadParser.

@ctb
Copy link
Member Author

ctb commented Oct 6, 2016 via email

@luizirber
Copy link
Member

Then we have a conflict with #1484, I had the impression we should keep an empty quality attribute?

@ctb
Copy link
Member Author

ctb commented Oct 7, 2016

Well, umm, I think (3) personally. Which fits with a lot of existing Python code. shrug It's up for discussion.

@luizirber
Copy link
Member

I agree with 3, but we need to discuss how similar we want both screed.Record and khmer.Read to be, and if we should then remove the quality attribute from khmer.Read if it doesn't have quality.

@ctb
Copy link
Member Author

ctb commented Oct 7, 2016

On Thu, Oct 06, 2016 at 05:36:04PM -0700, Luiz Irber wrote:

I agree with 3, but we need to discuss how similar we want both screed.Record and khmer.Read to be, and if we should then remove the quality attribute from khmer.Read if it doesn't have quality.

Yes, see #769. I personally think that the Python code is more likely to
reflect good syntax/use because it's easier to change, so would go with
screed's approach. But then again I've had more of a hand in screed's approach,
so that's probably to be expected....

Are there any drawbacks to the approach?

@luizirber luizirber mentioned this issue Oct 7, 2016
2 tasks
@betatim
Copy link
Member

betatim commented Oct 24, 2016

I think we can make khmer.ReadParser behave like option 3. I'd start with adding https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_getattro to the Read type that checks if the stored string is "" or not to decide.

@ctb
Copy link
Member Author

ctb commented Dec 19, 2016

This already works, c.f. #1559.

@ctb ctb closed this as completed Dec 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

3 participants