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

Bad concept creation #288

Open
hfaghihi15 opened this issue Sep 14, 2021 · 4 comments
Open

Bad concept creation #288

hfaghihi15 opened this issue Sep 14, 2021 · 4 comments
Assignees
Labels
document missing Something need more clear documentation Possible Bug

Comments

@hfaghihi15
Copy link
Collaborator

Hi,

As we previously had the notion of concept("str") to generate a named subconcept but @auszok has changed it to initializing a variable, we now get different results from concept1 = concept("concept1") and concept1 = concept(name='concept1') . I think we should fix and change this notion to avoid future problems.

@guoquan @auszok

@auszok
Copy link
Collaborator

auszok commented Sep 14, 2021

All the graphs where using "name" keyward like:
char = Concept(name='char')
word = Concept(name='word')
phrase = Concept(name='phrase')
sentence = Concept(name='sentence')

Thus I am using concept("x") in the new lc definition.

@hfaghihi15
Copy link
Collaborator Author

All the graphs where using "name" keyward like:
char = Concept(name='char')
word = Concept(name='word')
phrase = Concept(name='phrase')
sentence = Concept(name='sentence')

Thus I am using concept("x") in the new lc definition.

Yes but actually the name was optional and we mentioned this somewhere in the document. This may cause confusion also when people are trying to work with this. We can either come up with a name for the variable definition or mention this clearly in the documentation and remove the old notes.

@auszok
Copy link
Collaborator

auszok commented Sep 16, 2021

I checked the examples before I adopted the usage concept(x) for new lc. I believe most if not all used name when defining concept.
Actually all the example run.

Thus I will suggest that we remove the optional way of defining concept from documentation and required the usage of name, as we anyway did in our examples.

@hfaghihi15
Copy link
Collaborator Author

@AdmiralDarius Can you check your documentation for the graph concepts to make sure that it doesn't mention that the name property is optional? I remember that we mentioned it before so it may still be somewhere in your new doc.

@hfaghihi15 hfaghihi15 assigned DariusNafar and unassigned guoquan and auszok Oct 13, 2021
@hfaghihi15 hfaghihi15 added document missing Something need more clear documentation Possible Bug labels Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document missing Something need more clear documentation Possible Bug
Projects
None yet
Development

No branches or pull requests

4 participants