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

Next release of TranscodingStreams will likely break this package #118

Closed
nhz2 opened this issue Mar 29, 2024 · 1 comment · Fixed by #119
Closed

Next release of TranscodingStreams will likely break this package #118

nhz2 opened this issue Mar 29, 2024 · 1 comment · Fixed by #119

Comments

@nhz2
Copy link

nhz2 commented Mar 29, 2024

FASTX.jl/src/fasta/index.jl

Lines 230 to 234 in 868d702

# Disturbingly, there is no API to get the absolute position of
# an Automa machine operating on a stream. We ought to fix this.
# This workaround works ONLY for a NoopStream,
# and relies on abusing the internals.
buffer_offset = buffer.transcoded - buffer.marginpos + 1

Why does this code use the internals of TranscodingStreams?

I will soon remove the transcoded field from the TranscodingStreams.Buffer type, so this package will be broken.

@jakobnissen
Copy link
Member

TranscodingStreams was originally part of BioJulia - both this package and TS were created by a collaboration between Kenta Sato and Sabrina Ward. The original authors didn't pay too much attention to public vs. private interfaces of the packages, and happily pirated between packages and reached into internals.
There has been some effort to disentangle the different packages, but it's likely lots of internals remain.

Now that I have your attention - TS provide the buffer type, but this type and its fields are not listed as part of the API. However, there is a need for a package that provide these low-level details to the user. For example, the markpos field, IIUC, is needed precisely for users to mark and unmark positions in the buffer, such that the user can safely read that position from the .data field.
It might be possible to not reach into the internals of Buffer in FASTX.jl, but the package Automa.jl is much more intimately entangled with TranscodingStreams, and needs access to its internals. Automa, in turn, is used by quite a lot of packages. I do support drawing more clear lines between public and private, but in this particular case it needs to be done carefully, lest huge parts of the ecosystem break.

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 a pull request may close this issue.

2 participants