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

KeyError: 'label' error message could be more clear #25

Closed
lgmoneda opened this issue Dec 3, 2018 · 10 comments
Closed

KeyError: 'label' error message could be more clear #25

lgmoneda opened this issue Dec 3, 2018 · 10 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@lgmoneda
Copy link

lgmoneda commented Dec 3, 2018

When you miss declaring a node in your causal graph, it's going to throw a KeyError: 'label' error. It could be more explicit to make debugging easier. I think it would be nice to inform what is the node hough used in the graph.

@akelleh
Copy link
Contributor

akelleh commented Mar 7, 2019

Great suggestion. We might need a broader overhaul of logging and exception handling. What do you think @amit-sharma ?

@amit-sharma amit-sharma added the enhancement New feature or request label Jun 18, 2019
@amit-sharma amit-sharma added the good first issue Good for newcomers label Oct 26, 2019
@amit-sharma amit-sharma self-assigned this Feb 8, 2020
@yemaedahrav
Copy link
Contributor

Is this error caused when calling the 'CausalModel()' when it internally creates the graph through the 'CausalGraph()' API or somewhere else? because I am not able to reproduce the error locally.

@amit-sharma
Copy link
Member

I think this error is raised when you input your own graph as a string and forget to add all required nodes. So if you provide a dot graph with missing nodes, and then call CausalModel with that graph, it should throw this error.

@lgmoneda
Copy link
Author

Hi! I'm unsure if it persists since I'm not currently using the library. But the point was that the error message didn't help as much as it could in identifying what the missing nodes were.

@rahulbshrestha
Copy link
Contributor

Hey @amit-sharma! I would like to help out in fixing this bug. Can you let me know what kind of error message do you expect?

@amit-sharma
Copy link
Member

thanks for contributing, @rahulbshrestha It will be nice to output an error message that describes the source of the bug. Something like, "Some nodes are missing in the graph: {node1name, node2name} "

@rahulbshrestha
Copy link
Contributor

rahulbshrestha commented Apr 16, 2024

I can't seem to reproduce this issue, how can I provide a dot graph with missing nodes such that the KeyError appears?

Experimenting with this:

# With DOT string
model=CausalModel(
        data = df,
        treatment='X',
        outcome='Y',
        graph="digraph {Z -> X;Z -> Y;X -> Y;}"
        )
model.view_model()

where, df consists of X,Y and Z as columns.

I'm not sure if @lgmoneda meant in this case

# With GML string
model=CausalModel(
        data = df,
        treatment='X',
        outcome='Y',
        graph="""graph[directed 1 node[id "Z" label "Z"]
                    node[id "X" label "X"]
                    #node[id "Y" label "Y"]
                    edge[source "Z" target "X"]
                    edge[source "Z" target "Y"]
                    edge[source "X" target "Y"]]"""

        )
model.view_model()

where an NetworkXError: edge #1 has undefined target 'Y' error is thrown if we remove the node Y.

@amit-sharma
Copy link
Member

Try this. Does the following code raise the error?

model=CausalModel(
data = df,
treatment='X',
outcome='Y',
graph="digraph {Z -> X;}"
)
model.view_model()

@rahulbshrestha
Copy link
Contributor

Nope, it works perfectly fine
image

@amit-sharma
Copy link
Member

Thanks for checking @rahulbshrestha Looks like this issue is resolved in the current version. We can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants