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

additional metadata for attributes #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

abx78
Copy link

@abx78 abx78 commented Feb 10, 2020

support for additional metadata in the Enum keys

@abx78
Copy link
Author

abx78 commented Feb 10, 2020

it should fix issue #6

Copy link
Owner

@kindlychung kindlychung left a comment

Choose a reason for hiding this comment

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

Please document your changes in the docstring and create some tests in test/runtests.jl. Thanks.

@abx78
Copy link
Author

abx78 commented Feb 10, 2020

Sure, I will work on it.

@kindlychung
Copy link
Owner

I am not convinced that mixing the attributes with the string representation is a good idea. Would be nice if we have two separate operators:

  • => for giving a custom string representation, without attributes. This is the previous behavior.
  • -> for giving attributes, without a custom string representation. For example, if you define @se Team Barcelona->(country="Spain", revenue=840.8) RealMadrid->(id=3,country="Spain", revenue=757.3), string(Team.Barcelona) would be simply "Barcelona"
  • What if the user wants to have both attributes and custom string representation? We can allow a special attribute called repr, whenever the users gives that attribute, we use it as the string representation. For example, if you define @se Team Barcelona->(country="Spain", revenue=840.8, repr="BCLN") RealMadrid->(id=3,country="Spain", revenue=757.3), string(Team.Barcelona) would be "BCLN"

This will be a cleaner design IMHO.

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.

2 participants