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

Added compressToCustom and decompressFromCustom function #183

Merged
merged 9 commits into from
Jan 22, 2024

Conversation

HelloLudger
Copy link
Contributor

as discussed in / (maybe) useful for

It basically works similar to ...Uint8Array(), first compressing everything and then figuring out, how the UTF-16 characters can be stored with the provided dictionary(-string).

I couldn't make the _compress/_decompress route work, because the custom dictionary length is often not 2^something, so _decompress wouldn't work (or _compress would only use a subset of the provided characters).

@Rycochet
Copy link
Collaborator

Can this be updated to the new split file version - should be relatively easy to do - testing is getting improvements but the basic pattern is in there!

@HelloLudger
Copy link
Contributor Author

Yes, looking into it. Hopefully this weekend.

@HelloLudger
Copy link
Contributor Author

Hey, I hope this will work for you.

tests/testValues.ts Outdated Show resolved Hide resolved
@Rycochet
Copy link
Collaborator

Only thing blocking this is getting the tests to use the same boilerplate as elsewhere - trying to figure out how to add this to the commandline code (still working on that so don't worry about adding it yourself!)

@HelloLudger
Copy link
Contributor Author

I didn't use runTestSet because I don't know how to deal with the second parameter for the custom function (the custom dictionary). The "lazy" way would be to give them a default custom dictionary, then runTestSet could be used as is.

@Rycochet
Copy link
Collaborator

I didn't use runTestSet because I don't know how to deal with the second parameter for the custom function (the custom dictionary). The "lazy" way would be to give them a default custom dictionary, then runTestSet could be used as is.

I've replied on the thread, hopefully it makes more sense now :-)

@Rycochet
Copy link
Collaborator

Also note that we've got a consistent license header on all other files now, can you add that to your new ones :-)

@HelloLudger
Copy link
Contributor Author

HelloLudger commented Jan 22, 2024

Next try. Hopefully it works now. Found a missing typing for compressToCustom. Seems like TypeScript and testing is useful after all.

I tried to create a test case with an 13-character heterogram (unproblematic), but then the tests "Repeating String" and "Long String" fail, because the "compressed" string is longer than the original string. The custom dictionary has to have at least 16 chars to pass these tests.

Just FYI, for the testing it is not important.

Edit: And another missing typing... don't know why the error wasn't triggered eartlier...

Comment on lines +20 to +26
describe("23 letter heterogram", () => {
const heterogram = "BlockyDwarfZingsTheJump";
const compress = (input: string | null) => compressToCustom(input, heterogram);
const decompress = (input: string | null) => decompressFromCustom(input, heterogram);

runTestSet<string>(compress, decompress, "");
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is awesome!!

@Rycochet
Copy link
Collaborator

The size test is probably not important - I'm moving over to only testing known output and making sure that the raw data is hashed and known correct - should have that merged later tonight, which will give us a better testing base :-)

@Rycochet Rycochet merged commit 9811b2b into pieroxy:master Jan 22, 2024
2 checks passed
@marawannwh
Copy link

marawannwh commented Jan 31, 2024

Sorry, I am a little bit confused. How to use this modification? I thought it added compressToCustom and decompressFromCustom to the lib, but I just installed it, and these functions don't exist.

I want it mainly for this issue #182 (I mean if this modification solves it)

@Rycochet
Copy link
Collaborator

@marwan-nwh How did you install it? v2 (that this is part of) is not currently released to npm so you need to install manually.

@marawannwh
Copy link

@marwan-nwh How did you install it? v2 (that this is part of) is not currently released to npm so you need to install manually.

Got it, thanks!

@HelloLudger
Copy link
Contributor Author

Can you elaborate on the found problem? Anything I can do to help?

@Rycochet
Copy link
Collaborator

Rycochet commented Feb 5, 2024

@HelloLudger I've created a draft PR with more detail and an easier way to test it - #200 - hope that helps!

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 this pull request may close these issues.

3 participants