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

API review: an alternative namo for dnaio.open? #70

Closed
marcelm opened this issue Apr 8, 2022 · 2 comments
Closed

API review: an alternative namo for dnaio.open? #70

marcelm opened this issue Apr 8, 2022 · 2 comments

Comments

@marcelm
Copy link
Owner

marcelm commented Apr 8, 2022

I kind of like offering dnaio.open() as the main entry function, but the open() name of course collides with the built-in function of the same name. This prevents users from writing from dnaio import open. For reading, this is kind of ok, but for writing, it looks not so nice because you also need the SequenceRecord and then have two import lines:

import dnaio
from dnaio import SequenceRecord
with dnaio.open("out.fastq") as f:
  f.write(SequenceRecord("name", "ACGT", "####"))

Alternatively, one could write dnaio.SequenceRecord and avoid the second import, but as a user, I may not want the dnaio. prefix everywhere. One can also rename the open function when importing, but that’s also not optimal.

So perhaps we should offer the dnaio.open function under a different name.

@marcelm marcelm changed the title API review: an alternative to dnaio.open? API review: an alternative namo for dnaio.open? Apr 8, 2022
@rhpvorderman
Copy link
Collaborator

rhpvorderman commented Apr 8, 2022

In the projects that I use dnaio.open I think that works very well. Just like gzip.open, lzma.open etc. If fits very well with the style of python.

In practice, I always write SequenceRecord objects that I read or modified. I do not really see a use for constructing FASTQ files on the fly. I mean, in that case "@" + name + "\n" + sequence + "\n+\n" + qualities + "\n" and using a regular open function works much better anyway.

EDIT: Come to think of it, I only use dnaio.open for reading. I use regular xopen for writing and then use the fastq_bytes method.

@marcelm
Copy link
Owner Author

marcelm commented Apr 8, 2022

Ok, writing the tutorial (#72) just now, I think I’m coming to the conclusion that adding the dnaio. prefix in the rare cases it’s needed is acceptable. I believe what got me thinking about this was that I added type annotations to some functions in Cutadapt, and in those cases leaving out the dnaio. prefix would be a bit nicer.

But let’s close this for now. Adding an alias for open also increases complexity, which would be another reason against it.

@marcelm marcelm closed this as completed Apr 8, 2022
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

No branches or pull requests

2 participants