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

(name :under_scored) returns "under-scored"... Why? #1974

Closed
jedi2light opened this issue Feb 15, 2021 · 18 comments · Fixed by #1977
Closed

(name :under_scored) returns "under-scored"... Why? #1974

jedi2light opened this issue Feb 15, 2021 · 18 comments · Fixed by #1977

Comments

@jedi2light
Copy link

I have the next code:

(setv sample :under_scored)
(name sample)

Expected:

"under_scored"

Actually:

"under-scored"

At the same time, Clojure's namereturns"under_scored"`. Please, fix it.

@Kodiologist
Copy link
Member

name calls unmangle internally. name itself is of dubious value, though; we should probably just trash it.

@allison-casey
Copy link
Contributor

Could we just make an exception for keywords since they should probably stay as close to their string representation as possible anyways?

@Kodiologist
Copy link
Member

It would be better for the user to use whatever combination of str, mangle, unmangle, and (. … __name__) that they want. Again, I don't see what the value of name is to begin with.

@allison-casey
Copy link
Contributor

allison-casey commented Feb 15, 2021

because as far as i can tell none of those get you the underlying name of the keyword.

(str :hello_world) ; =>  :hello_world
(repr :hello_world) ; =>  HyKeyword('hello_world')
(mangle :hello_world) ; =>  hyx_XcolonXhello_world
(unmangle :hello_world) ; =>  :hello-world
(name :hello_world) ; =>  hello-world

The closest to what jedi wants is str but that doesn't remove the : and it's such a common operation that it doesn't seem right to make users do (.replace (str ':hello_world) ":" "" 1) every time or make their own function. Clojure doesn't unmangle keywords and we probably shouldn't either

@Kodiologist
Copy link
Member

You can use sym.name or (cut sym 1).

@allison-casey
Copy link
Contributor

lol didn't know that sym.name existed that changes things.
@jedi2light you should probably use (. :hello_!->world name) ; => "hello_!->world"
instead. since hy doesn't have namespaces like clojure i think i agree with kodi now. There doesn't seem much use for name besides providing an alias for (. key name)

@allison-casey
Copy link
Contributor

There is one use case I'm starting to think of and that's the ability to normalize dict keys to strings easily.

(setv x {"a" 1  :b 2})

(dfor [k v] (x.items) [(name k) v]) ; => {"a" 1 "b" 2}
;; you could write that as
(dfor [k v] (x.items) [(if (keyword? k) k.name k) v]) ; => {"a" 1  "b" 2}

Don't know how valid of use case this is to keep name around, but something to think about

@Kodiologist
Copy link
Member

Generally, it's a mistake to use a HyKeyword instead of a string as a dictionary key. It's too easy to confuse yourself, especially when you're looking at forms like (dict :foo 1).

@allison-casey
Copy link
Contributor

yeah, that's why i don't use them myself, but i'm trying to think of reasons to keep name around since hy doesn't have clojure style namespaces which clojure's name handles specially. So far i'm not comming up with much 🤷‍♀️

@scauligi
Copy link
Member

scauligi commented Feb 15, 2021

it's a mistake to use a HyKeyword instead of a string as a dictionary key

That seems to go against the intention of the HyKeyword.__call__ implementation, but I agree that keywords-as-keys gets confusing with keywords-as-keyword-arguments. Should we consider removing the (:foo mapping) feature? Or instead, we could change it to do a string lookup instead of a HyKeyword lookup (e.g. (:foo (dict :foo 1)) would produce 1).

@Kodiologist
Copy link
Member

Probably the latter.

@peaceamongworlds
Copy link
Contributor

Or instead, we could change it to do a string lookup instead of a HyKeyword lookup (e.g. (:foo (dict :foo 1)) would produce 1).

I think that string lookup would be the most convenient. It would actually allow for it to be used with other python objects like pandas dataframes etc. Using keywords outside of function arguments doesn't seem to mix well with the python ecosystem.

@allison-casey
Copy link
Contributor

allison-casey commented Feb 15, 2021

I like the string lookup idea. I pretty much never use the (:akey adict) type expression because keywords as dict keys is generally a bad idea anyways.

so would (:hello {:hello 1}) be a KeyError then?

@peaceamongworlds
Copy link
Contributor

so would (:hello {:hello 1}) be a KeyError then?

I guess it would just be equal to (get {:hello 1} (. :hello name)), which throws a KeyError.

@jedi2light
Copy link
Author

Remove (:some col)? Ok, will switch to another language then :/

@allison-casey
Copy link
Contributor

The current proposals would still work though? it would just be a lookup for a string key.
(:hello {"hello": 1}) ; => 1 So i'm confused?

@scauligi
Copy link
Member

And in particular, (:hello (dict :hello 1)) ; => 1 with the new proposal, which seems like a plus.

@scauligi
Copy link
Member

Awesome, looks like there's enough buy-in so I'll start up a PR for this.

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

Successfully merging a pull request may close this issue.

5 participants