Add caption support for datasources - #99
Conversation
Sorry, something went wrong.
t8y8
left a comment
There was a problem hiding this comment.
Nothing blocking, I think the test cleanup can definitely wait until a new PR. Do consider the 'removing caption' case though.
Sorry, something went wrong.
|
|
||
| @caption.setter | ||
| def caption(self, value): | ||
| self._datasourceXML.attrib['caption'] = value |
There was a problem hiding this comment.
Nit: We do .set everywhere else (or everywhere in connections)
Sorry, something went wrong.
| elif os.path.isfile(path): | ||
| os.unlink(path) | ||
|
|
||
| def get_temp_file(self, filename): |
There was a problem hiding this comment.
We have 3 ways of doing temp file management now.
xfile.temporary_directory has a context manager that creates and deletes a temp dir
bvt.py does a less advanced version of your cleanup methods here
Then these cleanup methods here.
This can be another PR, maybe we want to unify them, at least across the tests?
Sorry, something went wrong.
There was a problem hiding this comment.
Agreed, I'll do a pass to create a "temp file wrapper that both library code and tests can use" along with the auto clean up base test class.
Sorry, something went wrong.
| def caption(self): | ||
| return self._caption | ||
|
|
||
| @caption.setter |
There was a problem hiding this comment.
Are all datasources guaranteed to have a caption or would you ever want to remove it?
We could apply the None strategy like we do for port
Sorry, something went wrong.
There was a problem hiding this comment.
I think it's more idiomatic to do del ds.caption
Sorry, something went wrong.
|
🚀 |
Sorry, something went wrong.
Addresses #98