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

__contains__ pollutes __dict__ with empty Namespace values #14

Closed
christian-storm opened this issue Sep 10, 2018 · 4 comments
Closed

__contains__ pollutes __dict__ with empty Namespace values #14

christian-storm opened this issue Sep 10, 2018 · 4 comments

Comments

@christian-storm
Copy link

from pytool.lang import Namespace
f = Namespace({'a': {'a1': [1]}, 'b': {'b1': [2]}, 'c': {'d': [1, 2, 3]}})
print(bool('e' in f))  # False but creates f.e = Namespace()
print(bool('e' in f.__dict__))  # True

Doesn't it make sense that 'e' would only be added in assignment and not in attribute lookup/contains?

The reason I found this was because I added a method knitems

    for name, value in self.__dict__.items():
            yield name, value

to be able to iterate one level at a time. I've had to add

        if isinstance(value, Namespace) and bool(value):

in order to make it work.

@shakefu
Copy link
Owner

shakefu commented Sep 11, 2018

Good find. I'll see if I can get a patch fix published this week unless you want to PR earlier.

@christian-storm
Copy link
Author

I'm a bit flat out right now. I may be able to get to it in a week or so.

@christian-storm
Copy link
Author

This seems to have fixed things for me but wanted your opinion on whether this is the right fix or not.
Basically, if the contains is going to return False just delete the empty vestigial namespace created by getattr.

What do you think?

    def __contains__(self, name):
        names = name.split('.')

        obj = self
        for name in names:
            obj = getattr(obj, name)

        if isinstance(obj, Namespace):
            if obj.__dict__:
                return True
            else:
                del self.__dict__[name]  # <--- I added this line
                return False
        else:
            return True

@shakefu
Copy link
Owner

shakefu commented Apr 11, 2019

@christian-storm This is fixed in master. Apologies for the long delay, GitHub notifications suck.

@shakefu shakefu closed this as completed Apr 11, 2019
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

No branches or pull requests

2 participants