Skip to content

Should not generate error if attribute not found on python object#58

Merged
greatwolf merged 2 commits intobastibe:masterfrom
91Act:master
May 14, 2017
Merged

Should not generate error if attribute not found on python object#58
greatwolf merged 2 commits intobastibe:masterfrom
91Act:master

Conversation

@Jayatubi
Copy link
Contributor

IMHO, if the attribute is not found on python object it should just return nil otherwise it would break the entire lua state machine.

@greatwolf
Copy link
Collaborator

I'm unsure about this. In python, if you access a non-existing attribute eg. getattr(foo, 'bar') it would throw an AttributeError: object has no attribute 'bar'. Wouldn't it be more consistent to have it do the same on the lua side?

If nil is returned you'll always have to check for it on the lua side and if you want to error out at that point you won't have the python error string. OTOH you can just wrap this section of code in a pcall and handle it however you want; this will avoid the breakage you're talking about.

@bastibe @chrisju @sapir, what do you guys think of this PR's design change?

@bastibe
Copy link
Owner

bastibe commented May 12, 2017

What is lua's default if you access a non-existant key on a table?

If the answer is nil, how does Lua differentiate between a key that contains nil and a non-existant key? Maybe it doesn't make a difference. But then, Python certainly does make a difference between a non-existant key and a key that returns None.

@greatwolf
Copy link
Collaborator

A non-existing key is synonymous with a nil value in lua. In fact, to delete a key-value pair from a table, you just assign nil to it.

@Jayatubi
Copy link
Contributor Author

Access a non-existant key in a table in lua will return a nil value. No exceptions, no errors.

Lua 5.3.4  Copyright (C) 1994-2017 Lua.org, PUC-Rio
> t = {}
> print(t["no_exist"])
nil
>

@bastibe
Copy link
Owner

bastibe commented May 12, 2017

@Jayatubi what do you mean by

otherwise it would break the entire lua state machine.

@Jayatubi
Copy link
Contributor Author

Jayatubi commented May 12, 2017

@bastibe The error would case the lua enter a error state and force quit if there is no error handling. Just like you get exception in python or C. But normally a nil value is expected instead of a breakage.

class myclass:
    pass

import lua
func = lua.eval('function (obj) print(obj.foo) end')
instance = myclass()
func(instance)

Expect:

nil

But get:

Traceback (most recent call last):
  File "test.py", line 7, in <module>
    func(instance)
Exception: error: [string "<python>"]:1: unknown attribute in python object

@bastibe
Copy link
Owner

bastibe commented May 12, 2017

I haven't done Lua in a long time. Lua does not support exceptions, right?

@greatwolf
Copy link
Collaborator

greatwolf commented May 12, 2017

It does sort of by using pcall or xpcall. eg. the above could be handled like this:

func = lua.eval('function (obj)
                   local has_foo = pcall(function() return obj.foo end)
                   print(has_foo and obj.foo or nil)
                 end')

Admittedly, that's more code to handle the case. I guess the question boils down to how this should be handled, stay with python's way of handling errors by throwing or with lua by returning nil instead?

@greatwolf
Copy link
Collaborator

This raises an interesting design question. As I understand, and as you mentioned, python treats non-existing and None as two different things. If the lua way of handling this is used, how would these 2 cases be distinguished on the lua side?

@Jayatubi
Copy link
Contributor Author

Jayatubi commented May 12, 2017

In my experience when coding in lua it usually expects the non-existing to nil as default behavior. It's seldom to handle exception unless someone or some documentations explicitly mentions, such as invoke external APIs.

@bastibe
Copy link
Owner

bastibe commented May 12, 2017

It is then a question whether lunatic-python should represent Python objects as table-like or try to be as faithful to Python as possible. Personally, I prefer purity over convenience. After all, it would be relatively easy to prevent that error from Lua by calling hasattr first.

Opinions?

@Jayatubi
Copy link
Contributor Author

Jayatubi commented May 12, 2017

Generally lua doesn't have such hasattr method to tell if a key is in a table. Usually just do foo['bar'] == nil.

If the target object is a lua table there could be a wrapper such as:

function exists(table, key_to_check)
    for key, _ in pairs(table) do
        if key == key_to_check then
            return true
        end
    end
    return false
end

But this could only be used with native lua table, in other words support pairs, not for userdata:

bad argument #1 to 'for iterator' (table expected, got POBJECT)

@greatwolf
Copy link
Collaborator

I suppose we could take the spirit of "when in Rome" approach. IOW, do what the host language would expect. If using python in lua, lua code typically expects nil for missing keys so I can see returning nil for missing attributes making sense.

For cases where the user actually wants to distinguish between None and missing attribute errors, they can use hasattr from python as @bastibe mentioned.

We can also pose this question on say /r/python and /r/lua to get some feedback; incase there are other important considerations we're overlooking.

Actually push and return `nil`.
@greatwolf
Copy link
Collaborator

Slightly off-topic but @bastibe can you enable travis CI for lunatic under your account? This way, new PR merges can be executed automatically.

@greatwolf greatwolf merged commit ccc5241 into bastibe:master May 14, 2017
@bastibe
Copy link
Owner

bastibe commented May 15, 2017

@greatwolf Travis CI should be enabled now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants