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

Implement ToString for DS Dictionary #11358

Merged
merged 2 commits into from
Dec 11, 2020
Merged

Conversation

mmisol
Copy link
Collaborator

@mmisol mmisol commented Dec 11, 2020

Purpose

This is done to provide meaningful output when a dictionary is provided
to nodes like 'String from Array' and 'String from Object'. These nodes
call built-in DS functions that end up calling ToString on the
dictionary, which until now used the default implementation inherited
from Object which just returned the class name.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

This is done to provide meaningful output when a dictionary is provided
to nodes like 'String from Array' and 'String from Object'. These nodes
call built-in DS functions that end up calling ToString on the
dictionary, which until now used the default implementation inherited
from Object which just returned the class name.
@mmisol
Copy link
Collaborator Author

mmisol commented Dec 11, 2020

The format should be like this: {A:1,B:2,C:3,D:4}

Copy link
Contributor

@aparajit-pratap aparajit-pratap left a comment

Choose a reason for hiding this comment

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

@mmisol thanks for fixing this!

@mmisol mmisol merged commit 15e4aec into DynamoDS:master Dec 11, 2020
QilongTang pushed a commit that referenced this pull request Jan 27, 2021
This is done to provide meaningful output when a dictionary is provided
to nodes like 'String from Array' and 'String from Object'. These nodes
call built-in DS functions that end up calling ToString on the
dictionary, which until now used the default implementation inherited
from Object which just returned the class name.
@QilongTang QilongTang mentioned this pull request Jan 27, 2021
8 tasks
QilongTang added a commit that referenced this pull request Jan 28, 2021
* Dock and Undock view extension with Dynamic extension ID as edge case (#11394)

* Initial Commit with settings null check

* Add unit test

* Add view extension loading warning

* Update message

* Initial Commit (#11366)

* Do not aggregate node execution analytics anymore (#11356)

* Do not aggregate node execution analytics anymore

* Update to use new event combo

* Update Comments

* Make IntegerSlider the only AKA for migration (#11360)

There were two classes that were declaring an 'AlsoKnownAs' attribute
for 'DSCoreNodesUI.Input.IntegerSlider' in 'CoreNodeModels.Input':
'IntegerSlider' and 'IntegerSlider64Bit'. While the latter obsoletes
the former, it is not possible to migrate to it from XML, so only 
'IntegerSlider' should continue declaring an 'AlsoKnownAs' attribute.
Note that currently both classes are trying to be registered, which
results in only one of them being registered but not really making any
guarantees of which one it is. This also generates an exception catched
internally but shown in the console.

* Revised icons for AnyTrue and AnyFalse nodes (#11361)

* Implement ToString for DS Dictionary (#11358)

This is done to provide meaningful output when a dictionary is provided
to nodes like 'String from Array' and 'String from Object'. These nodes
call built-in DS functions that end up calling ToString on the
dictionary, which until now used the default implementation inherited
from Object which just returned the class name.

Co-authored-by: Martin Misol Monzo <[email protected]>
Co-authored-by: Laurence Elsdon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants