Skip to content
New issue

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

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

Already on GitHub? # to your account

represent encoded samples as separate type (EncodedSamples) #145

Open
ericphanson opened this issue Oct 11, 2023 · 3 comments
Open

represent encoded samples as separate type (EncodedSamples) #145

ericphanson opened this issue Oct 11, 2023 · 3 comments

Comments

@ericphanson
Copy link
Member

rather than having a flag in the struct itself. I think promoting to the type domain makes sense bc very little code is agnostic to whether or not it is encoded, and many methods only make sense for decoded ones. This can prevent a class of errors where the flag isn't checked when it should be (by instead getting MethodErrors). Also it is easy to add methods like f(s::EncodeSamples) = f(decode(s)) when desired.

@kleinschmidt
Copy link
Member

I think one potential downside to this is aliasing, since decode does not always allocate new storage (esp. if the offset/resolution are 0/1). So there would still be some footguns here, but still probably better than the status quo

@ericphanson
Copy link
Member Author

Imo aliasing is basically fine, it just has to be documented. Eg “!!! warning” with “Output may share data with input data” (ideally with exactly when if we can commit to something).

@jrevels
Copy link
Member

jrevels commented Nov 6, 2023

xref'ing #77 as breadcrumb for future me

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants