[rdf] Changes to core#113
Conversation
achilleas-k
left a comment
achilleas-k
left a comment
There was a problem hiding this comment.
I have a comment regarding setting IDs.
|
|
||
| @id.setter | ||
| def id(self, new_value): | ||
| self._id = new_value |
There was a problem hiding this comment.
Should the id setter check if new_value is a valid UUID4, or are we OK with it being anything?
More generally, do we even need a setter? I generally find that setting unique identifiers, especially ones that aren't exposed for normal use (i.e., we don't expect users to be manipulating IDs; they're just used internally for identification of objects), shouldn't be settable. They are generated on instantiation and beyond that they're read-only. There may be cases where setting is desirable, but I'd like to hear arguments for that. Also, since this is python, if one really really wants to set an ID (say, for testing) it's still possible by simply accessing the private member. By not having a property setter, we'd be signalling that IDs shouldn't be changed.
There was a problem hiding this comment.
I think you are right, we do not need the setter so far. While developing I used to think we would use it, but now it is clear that it is redundant.
There was a problem hiding this comment.
The only problem is that than the xmlparser should be modified. Since it uses setattr(obj, k, v), we should include special case for ids.
|
|
||
| after_success: | ||
| - if [ $COVERALLS = 1 ]; then coveralls; fi; | ||
| - if [ $COVERALLS = 1 ]; then coveralls; fi; No newline at end of file |
There was a problem hiding this comment.
Please keep the newline at the end of each file. :)
There was a problem hiding this comment.
Aaaah, thats not PEP8 conform, never mind then, my bad, ignore comment!
|
|
||
| def __init__(self, author=None, date=None, version=None, repository=None): | ||
| def __init__(self, author=None, date=None, version=None, repository=None, id=None): | ||
| super(BaseDocument, self).__init__() |
There was a problem hiding this comment.
any passed id is actually ignored, isn't it? Same for BaseSection/Document the id is always initialized with a fresh uuid.uuid4()
There was a problem hiding this comment.
Actually, yes, I think I should remove them from all constructors, since we are only passing ids through parsers. Am I right? I added them to constructor when I was using setters, so now they do not make sense. The general question though - could users actually create objects with their ids?
5 participants
This PR mainly includes some changes to doc, section, property and format modules.