Add human readable location data to picture metadata #197
Conversation
|
|
elodie.py
Outdated
| return True | ||
|
|
||
| config = load_config() | ||
| if ('Location' in config) and (config['Location'].getboolean('write_location')): |
There was a problem hiding this comment.
For consistency can you remove the parens?
if 'Location' in config and config['Location'].getboolean('write_location'):
elodie.py
Outdated
| if ('Location' in config) and (config['Location'].getboolean('write_location')): | ||
| # If config says to update locations | ||
| log.info(u'Human readable location = %s' % (location_name)) | ||
| print('Human readable location = %s' % (location_name)) |
elodie/media/media.py
Outdated
| if(not self.is_valid()): | ||
| return None | ||
|
|
||
| print('DEBUG - location = %s' % location) |
elodie/media/media.py
Outdated
| self.reset_cache() | ||
| return status | ||
|
|
||
| def set_address(self, location): |
There was a problem hiding this comment.
I think this should be named differently. We're specifically setting tags with address information, right?
I propose set_address_tags.
elodie.py
Outdated
| return True | ||
|
|
||
| config = load_config() | ||
| if ('Location' in config) and (config['Location'].getboolean('write_location')): |
There was a problem hiding this comment.
Instead of doing the check here I think it belongs in media.set_location. This way when you set the location on a file it will always add the tags.
There was a problem hiding this comment.
Not sure I follow you... I was thinking the idea was to not always add the tags, if that config option isn't set. Otherwise I don't need the config option.
| self.city_key: location['city'] | ||
| } | ||
|
|
||
| status = self.__set_tags(tags) |
There was a problem hiding this comment.
Can we or have we confirmed that this won't overwrite existing tags if they exist? We should include a test that covers this.
There was a problem hiding this comment.
Again, not sure I concur... If we are setting the value, shouldn't we be overwriting existing tags? If I can never overwrite a tag, then I can't "fix one" that's wrong.
Maybe I misunderstand.
|
Sorry for delay... kind of swamped with other things. I don't disagree that it needs tests added, but not sure I have the skills/understanding/time. If I get that far, I will, but wanted to at least get you this back. |
|
@bdillahu understandable about tests. I'll see if I can get to tests as well. Always appreciate PRs submitted early so they can be iterated upon. |
jmathai
force-pushed
the
master
branch
4 times, most recently
from
3276ccf to
12c17c9
Compare
jmathai
force-pushed
the
master
branch
2 times, most recently
from
bbdb460 to
ec11497
Compare
4 participants
Proposed addition to address Issue #195
Adds an option to enable this feature.