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

BaseField instantiate=True seems to fail #36

Open
djmoreho opened this issue Jan 12, 2016 · 3 comments
Open

BaseField instantiate=True seems to fail #36

djmoreho opened this issue Jan 12, 2016 · 3 comments

Comments

@djmoreho
Copy link

Creating a instance of a field will make it look like it fails because BaseField prints None, however it doesn't (I just spent a while seeing why it was 'failing').

Script that demonstrates the 'fail'

from suitcase.fields import UBInt8

# create our placeholder
u = UBInt8(instantiate=False)
print u # correct result

# create our actual object
u = UBInt8(instantiate=True)
print u # doesn't return a instance..... or does it

Fix would be simply changing the BaseField repr to,

def __repr__(self):
    return "Field(_value="+repr(self.getval())+")"

and we get a much better result that doesn't seem to fail, though having just the normal repr would give better information (i.e what field).

@posborne
Copy link
Contributor

@djmoreho What is your use case? There is some hackery with FieldPlaceholder and the like that were what I came up with at some point for supporting the primary use case (use of fields within the context of structures). Knowing your use case might help an opportunity for a suggestion on a better way to solve your problem or provide input for improving the design of suitcase for your use case.

@djmoreho
Copy link
Author

My use case was simply exploring the instantiate=True argument. When I was creating it at a python interactive prompt I ran into the repr None 'issue'. I mainly brought up the issue so if others run into it there is documented information. It isn't an actual issue (as the code works) but more a design decision.

@rcfox
Copy link
Contributor

rcfox commented Jan 22, 2016

I think part of the problem is that the fields' values default to None. I just submitted a PR in #37 to address that. (Though, it wasn't specifically because of this issue.)

Taken on their own, it isn't clear that these fields are wrapping the value their __repr__ returns, but I'd prefer to leave the __repr__ as it is. It makes it much nicer to look at a complete structure.

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

3 participants