Calling HyKeyword now looks up by string name, also remove name from hy.core

[scauligi]

Closes #1974.

[Kodiologist]

Why the change to __hash__?

You can write code like (. magic name) more concisely as magic.name.

[scauligi]

Why the change to __hash__?

This prevents (hash :kw) from producing the same value as (hash "kw"); it's a minor change but we should be doing this for "proper" hashing.

You can write code like (. magic name) more concisely as magic.name.

Done!

[Kodiologist]

we should be doing this for "proper" hashing

Why do you think so? The Python language reference says "The only required property is that objects which compare equal have the same hash value", not the converse. And there is no requirement that values of one type have different hashes from those of another type. For example, 1.0 and 1 both have the same hash, namely 1, and 1j has the same hash as 1000003.

[allison-casey]

So then #{"hello" :hello} ; => #{"hello"}. Which i guess makes sense if we're going for (dict :a 1) ; => {"a" 1} and (:a (dict :a 1)) ; => 1 here. How close do we want to align keywords and strings then?

[Kodiologist]

So then #{"hello" :hello} ; => #{"hello"}.

Not so. They're still not equal.

[allison-casey]

huh, good point. I thought dictionary keys/sets were based on hash not equality. I guess that's only their hashability and not their identity as far as python is concerned.

[Kodiologist]

It's not documented well, but my impression is that the hashes of objects only affects the performance of dictionaries, not their semantics: you could have a dictionary of 1,000 non-equal objects with equal hashes and everything would work as you expect. The weird part, which is documented, is that an object needs to be hashable in order for the object to be allowed as a set element or dictionary key in the first place.

[scauligi]

Yeah it's not for true correctness, technically it's just a mild but easy performance optimization.
Ideally if you have a != b then you'd like to have (but it's not necessary to have) hash(a) != hash(b), so that keys/sets can more effectively bucket things. A dictionary/set will still function properly even if you define every object's __hash__ to just return 1.

[Kodiologist]

So are you going to undo it?

[allison-casey]

If there's a performance gain for something that small I don't see why not. Is there a reason not to do it?

[Kodiologist]

I would need to see clear evidence of a performance improvement in realistic code before approving a change that's intended to improve performance. I really doubt this will make a difference.

[scauligi]

Sure I can remove it, it's not something I feel strongly about. Just something I noticed and figured might be a nice-to-have while I was in the nearby code.

[Kodiologist]

Looks good, thanks. Would you please:

• Update the relevant bit in syntax.rst, about (:foo obj).
• Mention the .name attribute in internals.rst, since it's now officially meant for use in Hy programs instead of just internal use (yes, "internals.rst" was a bad name).

[scauligi]

Done, as well as removing a couple references to the name function that I caught.

[Kodiologist]

Thanks, Sunjay. In the future, before merging a PR (your own or somebody else's), could you rebase it onto master first?

[scauligi]

Oh yes, my bad. I will be sure to do that going forward!