-
Notifications
You must be signed in to change notification settings - Fork 668
Separate associative arrays into Dictionary, List types #8194
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
Conversation
src/DynamoCore/packages.config
Outdated
| <packages> | ||
| <package id="Greg" version="1.0.6176.18754" targetFramework="net45" /> | ||
| <package id="Newtonsoft.Json" version="8.0.3" targetFramework="net45" /> | ||
| <package id="NUnit" version="3.8.1" targetFramework="net45" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this package added to Dynamo core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, weird.
|
Sounds good to me |
|
Cool, so this is simply to remove the entrypoint to dictionaries (not their existence), right? |
|
@kronz @Racel This is being pursued as an experiment at present. I'd really appreciate if you can read up on lua's tables, because it might explain some of the rationale for associative arrays in DS: https://www.lua.org/pil/2.5.html |
|
So, tables in Lua seem pretty cool, and there defintely seems like a common rationale. I'm with Racel, that a cleaner UX around list v dictionary behavior through seperation still seems like a better approach. "Simple, Coherent, Capable". Tables in Lua certainly seem Capable, but not simple. BTW, just for fun, if you put some of the same code in identified in the link into a CBN, you get some pretty nutty results: |
pboyer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aparajit-pratap I can't approve of this PR because I opened it, but I do approve. I have a few minor comments.
I have some broader comments, but I'll try and address them with a PR for you instead once this is merged.
| class ArrayMarshaler : PrimitiveMarshler | ||
| { | ||
| private FFIObjectMarshler primitiveMarshaler; | ||
| private readonly CLRObjectMarshler primitiveMarshaler; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a follow up PR, we should definitely fix these spelling errors. :D
| </data> | ||
| <data name="FailedToConvertArrayToDictionary" xml:space="preserve"> | ||
| <value>Cannot convert List to Dictionary type.</value> | ||
| </data> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's replace Array with List in InvalidArrayIndexType and FailedToConvertArrayToDictionary
| @@ -0,0 +1,59 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine. I think it's clear that one contains builtin DesignScript code and the other is builtin code contained in an assembly.


Purpose
This PR removes the ability to index arrays by anything but an integer. This is the first step in an separatio between array and dictionary behaviors in DesignScript's built-in types.
Currently, the language implements associative arrays quite similarly to Lua's Tables:
https://www.lua.org/pil/2.5.html
This branch makes the language more similar to two of the most popular scripting languages: Python and JavaScript. Both of these languages make a clear delineation between these two types.
FYIs
@ke-yu @Racel @aparajit-pratap