Schemata for the msd tagtraum parser in jams-data.

[hendriks73]

Hey Bryan,

these are two schemata I need for the simple msd_tagtraum_cd dataset support. I assume no other change is necessary, to have them be found, right?

Cheers,

-hendrik

[bmcfee]

Thanks!

They should be found automatically on load, but testing makes perfect!

A few comments:

1) Please implement unit tests for the new namespace definitions. If you're unsure of how to do this, check the gtzan tests for an example.

2) Is there a reason to keep these capitalized?

3) I noticed that some of the terms have underscores (Pop_rock) while others have spaces (New Age) and others are abbreviated (RnB). If at all possible, I'd like these things to be standardized within a namespace.

[hendriks73]

hey!

1. ok
2. no
3. if you’re keen on that I can fix it in both the parser and the namespaces. tomorrow.

it’s 7pm here. i’m done for the day!!

-hendrik

On Nov 5, 2015, at 19:00, Brian McFee notifications@github.com wrote:

Thanks!

They should be found automatically on load, but testing makes perfect!

A few comments:

1) Please implement unit tests for the new namespace definitions. If you're unsure of how to do this, check the gtzan tests https://github.com/marl/jams/blob/master/jams/tests/namespace_tests.py#L488 for an example.

2) Is there a reason to keep these capitalized?

3) I noticed that some of the terms have underscores (Pop_rock) while others have spaces (New Age) and others are abbreviated (RnB). If at all possible, I'd like these things to be standardized within a namespace.

— Reply to this email directly or view it on GitHub https://github.com/marl/jams/pull/83#issuecomment-154138889.

[hendriks73]

Done.

Another quick note about '_' vs ' ' (3): The underscore was an escape for '/', while the space is just that. A space. So there is a reasonable, but not obvious explanation for the difference.

I guess it makes sense to convert it back to '/' and ' ' to make it clear.

Cheers!

[bmcfee]

This looks good to me.

Last request! Can you add entries for these namespaces in the documentation? docs/namespaces/tag.rst is the relevant file to update. Ideally, the docs here should include a reference or link to the source of the definitions.

After that, I think it's merge-ready.

[hendriks73]

Can you add entries for these namespaces in the documentation?

Sure. Done.

Just FYI: Building the docs locally failed at first, because of a missing/broken six dependency. pip install mock==1.0.1 made it possible to build.

[bmcfee]

Thanks! Can you include the exact error message from the build so that I can fix it properly?

[hendriks73]

I simply ran make text from the command line. Perhaps this kind of error does not occur, when calling setup.py

Mankell:docs hendrik$ make text
sphinx-build -b text -d _build/doctrees   . _build/text
Running Sphinx v1.3.1
making output directory...

Exception occurred:
  File "/Library/Python/2.7/site-packages/mock/mock.py", line 68, in <module>
    from six import wraps
ImportError: cannot import name wraps
The full traceback has been saved in /var/folders/20/sc3z_ws14mj5zdf1rf6hql7w0000gn/T/sphinx-err-lA7NtB.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
make: *** [text] Error 1

Before that I installed Sphinx manually, i.e. with

sudo pip install -U Sphinx

Sphinx complained about Mock missing, so I installed mock-1.3.0 (apparently the latest version). Then Sphinx complained about Six, so in installed six-1.10.0 (apparently the latest version). Still Sphinx complained about...

Exception occurred:
  File "/Library/Python/2.7/site-packages/mock/mock.py", line 68, in <module>
    from six import wraps
ImportError: cannot import name wraps

After downgrading Mock to 1.0.1, it finally worked.

So I'm guessing, that Mock 1.3.0 needs a version of Six that actually has this wraps thing. Mock 1.0.1 apparently does not use Six at all, which is why it works. Dependency hell...

Perhaps you want to add a very short How to build section to the README.md file.

[bmcfee]

Oh man. sphinx 1.3 give me nightmares. Mock also.

Let's move this discussion over to a new issue. Meanwhile, the docs subdirectory does include a requirements.txt that should contain dependencies that are only needed for docs. This is only used automatically by readthedocs (which also fails on this project because of sphinx 1.3 badness), but i think any changes to get things working should be added there.

[hendriks73]

You mean like this:

{"tag_msd_tagtraum_cd1":
    {
        "value": {
            "enum": [
                "reggae",
                "pop/rock",
                "rnb",
                "jazz",
                "vocal",
                "new age",
                "latin",
                "rap",
                "country",
                "international",
                "blues",
                "electronic",
                "folk"
            ]
        },
        "confidence": {
            "oneOf": [ {"type": "number", "minimum": 0.0, "maximum": 1.0},
                       {"type": "null"} ]
        },
        "dense": false,
        "description": "msd tagtraum cd1 13-class genre annotation"
    }
}

[bmcfee]

Yup! And, because I'm a stickler, unit tests to match and verify that values outside that spec throw SchemaErrors.

[hendriks73]

Alright, Mr. Stickler... Are we finally done now?! ;-)

[hendriks73]

Cool. Thanks for taking the time to review stuff to make this good.