AttributeError when changing date tag

[dirtdigger]

Hello! New beets user, and I am working on slowly importing my music into beets.

I can't actually change the date on any of the files that I've already imported. I started by trying to change the date on an album whose original release date was wrong on MusicBrainz. Here's the command I used:

beet -v modify The Autumn Effect date="2005-08-16"

and the output: https://gist.github.com/dirtdigger/aea6c0c97d5bfbae37ac

I tried rolling back to 1.3.10 in case it was something new, but the same thing happened there too. It doesn't just happen with that album, it's happened to any album that I try to change the date on, and it happens on both flac and mp3 albums. This also happens when I just try to change the date separately.

System information: beets 1.3.11, Arch Linux

Any help would be appreciated. Thanks :-)

[tomjaspers]

Confirmed to be a problem on HEAD too. The date parsing isn't attempted anywhere, but I'm not sure at which level/step it should happen ideally.

[sampsyo]

Beets doesn't use a field called date (by default, at least). There are year, month, and day for the release date. And there's an added field which is a full date, but that's different from the release date itself.

Can you try modifying year, month, and day individually?

[sampsyo]

(That said, this definitely should not crash. 😃 I'll look into it.)

[dirtdigger]

Thanks for your reply!

I'm sorry, I read over what I originally wrote and realized it wasn't clear. When I said I tried to change the date separately I meant that I tried to change the year/month/day fields separately. Bottom line, I get the same output when I try to modify the year or month separately too :-(

beet -v modify The Autumn Effect day="16" yields this output: https://gist.github.com/dirtdigger/d1845971ce229d03ddff

For what it's worth, I was able to find a temporary workaround for my use case. The issue was that the Italian iTunes reports this as being released on January 1, whereas the album really wasn't actually released until August 16. This makes MusicBrainz (incorrectly) report 2005-01-01 as the original release date, so I just disabled original_date temporarily and re-imported :-)

[brunal]

@sampsyo: "date" is not an item field, and therefore absent of Item._types and Item._fields. Therefore beets.dbcore.db.Model._type() will fallback to types.DEFAULT (library.DateType would be expected). However date is supported by MediaFile: there are several DateField (date and original_date). We try to set its value with a string, which is not expected. I'm not sure when conversion should happen: should it be added as an element of Item._types or Item._fields? I'd go for the latter.

Moreover I built the following test in tests.test_ui.ModifyTest:

    def test_date_setting(self):                                                
        self.modify("date='1970-01-01'")                                        

It fails that way when run with the nosetests command:

Traceback (most recent call last):
  File "/home/bru/code/python-venvs/beets2/bin/nosetests", line 9, in <module>
    load_entry_point('nose==1.3.4', 'console_scripts', 'nosetests')()
  File "/home/bru/code/python-venvs/beets2/lib/python2.7/site-packages/nose/core.py", line 121, in __init__
    **extra_args)
  File "/usr/lib64/python2.7/unittest/main.py", line 95, in __init__
    self.runTests()
[...]
  File "/home/bru/code/python-venvs/beets2/lib/python2.7/site-packages/nose/plugins/capture.py", line 112, in _get_buffer
    return self._buf.getvalue()
  File "/usr/lib64/python2.7/StringIO.py", line 271, in getvalue
    self.buf += ''.join(self.buflist)
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 13: ordinal not in range(128)

Running it with python gives OP's error without any non-ascii character, so this might be a side effect of another test but running nosetests with that test only still shows the problem: $ nosetests -x test/test_ui.py:ModifyTest.test_date_setting

[sampsyo]

@dirtdigger Oh dear—now it looks like we've gotten your database into a bad state where every modify you do will re-try setting the underlying metadata field and get this error. That's no good at all. Fortunately, the fix will still be the same.

@brunal Thanks for digging into this. This matches my intuition.

One easy but unsatifying solution would be to make those date field names not propagate to MediaFile. That is, they would look like any other flexible attribute. (We could even remove them from MediaFile; they're not all that useful).

I suppose it would be nice to just make them do the expected thing, however: to implicitly set the component fields in the database. But then we'd need to build the opposite direction as well: updating the component year, month, and day should affect the date. That would mean the field would act as a computed field (as opposed to fixed or flexible) attribute. That, in turn, would require making computed fields settable—they aren't yet.

So I think I lean tentatively toward just removing date and original_date from MediaFile for now. They were a nice idea but not, in practice, all that useful. And if we build the "proxy" date field at the Library level, we won't even need it at the MediaFile level anymore!

That logging capture error is somewhat disturbing. I agree that it might be an artifact of something else. I wish those errors would blame the specific logging statement that caused them.

[flight16]

How will this affect being able to set an album's release date? I'm not sure what ID3 field it uses, but in all taggers and other players there is a field for date (what is being discussed in this issue, I assume). I will still be able to set "year" using beets, have it saved to the file, and view it in another player, right?

[brunal]

If I'm right:

• in beets/mediafile.py lines 1119 to 1244 and 1290 to 1712 should be removed, and {original_,}{day,month,year} added idependently
• in beets/library.py, Item._types should be extended to include {original_,}date.

Is that right? if so what type should those receive? a DateType-like instance with its parse(string) method returning a descriptor? The idea would be that setting date should set the 3 corresponding attributes. This asks for a descriptior with __set__ (and __get__) defined. However date won't be directly set: it will be instantiated with the string value at a point where the Item itself is not available: in the aforementioned parse method. Am I mistaken? Any idea how to circumvent that problem?

[sampsyo]

@flight16 It won't; no worries—modifying the components will continue to work.

@brunal Hmm; good point. I temporarily forgot that the aggregate date field was the "source of truth," so just a plain removal will not be straightforward. It's probably not worth it to remove that field.

So we have two options:

• Just "disconnect" the Item date field from the MediaFile as a special case. Make it an ordinary flexible field with no backing media field. We can make the field do the right thing later.
• Do the right thing now: make it a computed field. This will also disconnect the field from MediaFile.

If we do the latter, I think this will actually have less to do with DateType and more to do with the way computed fields work. Currently, they're plumbed with a simple "getter" function that the Model class can register. We'd now need to make this a getter/setter pair, akin to a Python descriptor. Then, the parsing can be decoupled from the logic here that aggregates and disaggregates the parts of the date.

[sampsyo]

Fixed the "easy" way. Thanks for your patience.