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

Revisit strictness of Access module #6957

Closed
wants to merge 1 commit into from
Closed

Revisit strictness of Access module #6957

wants to merge 1 commit into from

Conversation

amandasposito
Copy link

This PR aims to solve the issue #6515, that opened a discussion about lifting the restriction for the Access module, so it could interact with non-tuple lists.

I removed the call to the Keyword.get_and_update and Keyword.pop functions and implemented a solution closer to the Keyword private functions, just handling the new scenario for non-tuple lists.

The reason why I didn't implement a partial solution, using both Access and Keyword, was because it could cause a performance issue.

@umamaistempo
Copy link
Contributor

umamaistempo commented Oct 20, 2017

Since this does not requires the list to be a keyword anymore, i'd say to add tests using binary keys 😁

eg:

assert [{"foo", 1}, {"bar", 2}]["foo"] == 1

@ericmj
Copy link
Member

ericmj commented Oct 21, 2017

Thanks for the PR. I believe we are still discussing if we are going to support non strict keywords and in what way we are going to do it so we will hold off on merging this PR until we have a decision.

@ericmj
Copy link
Member

ericmj commented Oct 21, 2017

Since this does not requires the list to be a keyword anymore, i'd say to add tests using binary keys

We are not planning to extend keywords in this way. You should use maps for this instead.

@josevalim
Copy link
Member

We are not planning to extend keywords in this way. You should use maps for this instead.

It is still true though that this is no longer a keyword so there is no reason to also not support binaries. I am not saying we should, but it would be a valid direction.

@lexmag lexmag changed the title Revisit strictness of Access module #6515 Revisit strictness of Access module Apr 30, 2018
@josevalim
Copy link
Member

Hi @amandasposito!

I am so sorry but we have decided to not move this forward and keep Access semantically close to Keyword instead of introducing a third construct.

Apologies for the long time it took us to address this. ❤️

@josevalim josevalim closed this May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants