-
Notifications
You must be signed in to change notification settings - Fork 10
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
Asig metadata #15
Comments
Please take a look at the new branch feature-metadata, which implements a first version of a Metadata class.
Pros:
Cons:
Summary:
Next Steps:
|
Namingjust a thought. maybe Metadata objectMetadata could be moved to a separate file to declutter Asig.py. As it is a pure helper class I'd make it less verbose: import time
class Metadata(dict):
__getattr__ = dict.get
__setattr__ = dict.__setitem__
__delattr__ = dict.__delitem__
def __repr__(self):
res = "Asig metadata:\n"
for attr, value in self.items(): # sorted(self.items()) if keys should be in alphabetical order
if value:
if attr == 'timestamp':
value = time.strftime("%Y-%m-%d %H:%M:%S", time.localtime(value))
res += f" {attr:>15} = {value}\n"
return res
md = Metadata(foo="bar", bar=1, timestamp=time.time())
md.baz = 2.4
print(md) In this less verbose form with less than 20 LOC it may also stay in Asig.py. If possible do not name variables |
Three years later I'd probably use dataclasses for Metadata. Also we might want to introduce proper typing when we are at it. Python 2 support should not be an issue any longer (or is it)? |
wow. three years it is! tempus fugit. |
For Arecorder.record(), it would be useful to store the time stamps when recordings were made. One way would be to change
recordings.append((timestamp, asig))
, but that would make access and interpretation less intuitive. So that lead me to the idea of Asig.metadata:I propose to add an attribute Asig.metadata, which could be a dict with fields such as
By the way, Asig.label would be a perfect item in the meta data dict!
Before such a far-reaching change, I'd like to hear your feedback and ideas on this issue.
Note that we already have a context variable named '_' (yes, funny name, but short and nice, e.g. to access results of
a1.find_events().plot()
viaa1._['events']
andself._['plot']
.This is convenient for returning auxilliary data without breaking the logic of returning self always.
Hand in hand with that, I feel that the currently often used strategy to return a new Asig with new data
return Asig(newsig, self.sr, self.label, self.cn,...)
is suboptimal as it often fails to copy '_' and would, if metadata should be implemented, become even more complex. So probably a static method Asig._copy() for devs, which generally duplicates self w/o copying sig would help, so instead we could simply write
return Asig._copy(self, newsig, attr_to_overwrite=new_value,...)
Looking forward to hear your thoughts on both issues.
The text was updated successfully, but these errors were encountered: