Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

#156 Hashing implementation #158

Merged
merged 9 commits into from
May 24, 2023
Merged

Conversation

maxonfjvipon
Copy link
Member

@maxonfjvipon maxonfjvipon commented Feb 28, 2023

Closes: #156

@yegor256 WDYT about such algorithm?

The results:

  • hash TRUE = 72057594037927944
  • hash FALSE = 8
  • hash 42 = 3026418949609750528
  • hash 14325.2 = 1300271141437013430
  • hash "Hello world" = 7535155495499350018

PR-Codex overview

This PR introduces a new hash-code-of object to replace all as-hash objects in eo-collections.

Detailed summary

  • Adds hash-code-of object to eo-collections
  • hash-code-of replaces all as-hash objects
  • Adds tests for hash-code-of object in hash-code-of-tests.eo

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@yegor256
Copy link
Member

yegor256 commented Mar 1, 2023

@maxonfjvipon I love it, but would be great if you can add a comment/doc to the hash object. Now it's not clear what is it, how it works, and what it's for.

@maxonfjvipon
Copy link
Member Author

maxonfjvipon commented Mar 1, 2023

@yegor256 sure, btw, maybe the proper name would be hash-code (or hash-code-of) because hash is usually string? WDYT?

@yegor256
Copy link
Member

yegor256 commented Mar 1, 2023

Screenshot 2023-03-01 at 17 16 07

@maxonfjvipon let's call it hash-code-of

@maxonfjvipon
Copy link
Member Author

@yegor256 please have a look, something's wrong with build

@maxonfjvipon
Copy link
Member Author

@yegor256 previous hashing implementation for some reason takes about 30 seconds on my laptop to calculate hash code of just number. That's too much so I kind of copied and united current hashing implementations from eo-runtime

@maxonfjvipon
Copy link
Member Author

maxonfjvipon commented Mar 2, 2023

@volodya-lombrozo is there the same problem as was in eo-runtime?

@volodya-lombrozo
Copy link
Member

@maxonfjvipon Yes. Will be fixed soon: #160

@maxonfjvipon
Copy link
Member Author

@volodya-lombrozo seems your fix did not help

@volodya-lombrozo
Copy link
Member

@maxonfjvipon because @yegor256 returned the bug back - #161 :)

Ok, will fix rultor soon and hope it won't happen again. You can check progress here.

@yegor256
Copy link
Member

yegor256 commented Mar 3, 2023

@volodya-lombrozo it's not me, it's my script :) Find a way to prevent this bot to submit pull requests with this particular change. There has to be a way.

@cr-gpt
Copy link

cr-gpt bot commented Mar 24, 2023

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables for this repo. you could follow readme for more information

@pr-codex
Copy link

pr-codex bot commented Mar 24, 2023

Tldr

The PR introduces a new hash-code-of object and replaces all as-hash object usages with it in order to make eo-runtime independent of eo-collections.

Detailed summary

  • A new hash-code-of object is introduced.
  • All as-hash object usages are replaced with the new hash-code-of object.
  • The hash-code-bools-is-integer test is added to verify that the hash codes of boolean values are integers.
  • The hash-codes-of-the-same-numbers-are-equal test is added to verify that the hash codes of the same numbers are equal.
  • The hash-code-of-the-same-strings-are-equal test is added to verify that the hash codes of the same strings are equal.
  • The hash-code-of-different-numbers-are-different test is added to verify that the hash codes of different numbers are different.
  • The hash-of-int-is-int test is added to verify that the hash code of an integer is an integer.

@volodya-lombrozo
Copy link
Member

volodya-lombrozo commented Mar 24, 2023

@maxonfjvipon I'm afraid it will fail. Could you repeat that changes, please? https://github.com/objectionary/eo-collections/pull/160/files

We added a new feature to Rultor that prevents that changes and I hope it won't happen again.

@cr-gpt
Copy link

cr-gpt bot commented Mar 24, 2023

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables for this repo. you could follow readme for more information

@maxonfjvipon
Copy link
Member Author

@volodya-lombrozo it seems windows build still does not work

@maxonfjvipon
Copy link
Member Author

@yegor256 please, have a look

@yegor256
Copy link
Member

@volodya-lombrozo please, review this one

Copy link
Member

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@maxonfjvipon Can we add more tests for that object? It would be interesting to test floating numbers and, maybe, big numbers (or some corner cases).

src/test/eo/org/eolang/collections/hash-code-of-tests.eo Outdated Show resolved Hide resolved
@maxonfjvipon
Copy link
Member Author

@volodya-lombrozo I moved the test and added todo to add more tests in the future. WDYT?

Copy link
Member

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@maxonfjvipon Looks good to me. Thanks.

@maxonfjvipon
Copy link
Member Author

@yegor256 please merge

@yegor256
Copy link
Member

@rultor merge

@rultor
Copy link
Contributor

rultor commented May 24, 2023

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 548fa9c into objectionary:master May 24, 2023
@rultor
Copy link
Contributor

rultor commented May 24, 2023

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 8min)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hashing mechanism
4 participants