[WIP] [#604] Normalize organisations names

[felipevieira]

opentrials/opentrials#604

[victor-nitu]

So, played a bit with the failing build issue, here are some observations:

• Invoked without cache (pip install --no-cache-dir -r requirements.txt) it fails around fastcluster due to not finding numpy.
• Calling pip install -r requirements.txt uses the pip cache, so once the issue was fixed (see below) locally it won't reproduce anymore.
• Prior numpy installation with pip seems to fix it (just numpy, for now).
• Installing line by line fixes it. Two ways to do it:

  • iterate over package names in requirements.txt:

    $ for line in `grep -o '^[^#]\{1,\}' requirements.txt`; do pip install $line; done

  • iterate over requirements.in then do the remaining requirements.txt normally:

    $ while read p; do pip install --no-cache-dir $p; done < requirements.in
    $ pip install --no-cache-dir -r requirements.txt

There must be a pip gotcha I'm missing, though. Whoever finds the elegant fix for this (or the root cause, for that matter), please let me know 😉

[felipevieira]

Just for the record, this discussion summarizes the issue we are facing here. Apparently the most elegant solution comes from the libs (fastcluster and pyhacrf) which shouldn't use numpy as requirement on their setup.py

@nightsh both workarounds you've suggested work, and if we choose to go for any of them, I would also suggest to avoid the tox deps and add the custom command on the commands target, right before the tests itselves

[vitorbaptista]

Apart from my comments, could you explain why we need a training file? I thought we would simply read the contents from the DB when re-generating the clusters.

[vitorbaptista]

Instead of binding the organisation parameter via string interpolation, rely on SQLAlchemy for that. The query would become SELECT canonical FROM organisation_clusters WHERE '?'=ANY(variations) and you'd call it as conn['warehouse'].query(CLUSTER_QUERY, organisation) (if I remember correctly)

[felipevieira]

Resolved

[vitorbaptista]

Forgot to remove the pass

[felipevieira]

Resolved

[vitorbaptista]

These variations feel like should be in different clusters, as they look like different organisations.

[vitorbaptista]

I think this test is doing too much. First of all, the pattern of the organisation_cluster fixture is confusing, as it simply adds two fixtures to the DB without returning anything (which is probably the reason you've used @pytest.mark.usefixtures instead of adding it as a parameter). There's also the issue that we're hardcoding the data we expect to be created by the fixture here. I think a fixture is the wrong abstraction here, at least with the very simple fixture tools we have.

Instead of doing this, please remove the organisation_cluster fixture and create the data you need manually in the tests for the 3 cases you're testing. You could create a _create_organisation_cluster(self, conn, canonical_name, variations) to help keep the code DRY.

[felipevieira]

Resolved. Please check if the current approach meets your suggestion

[felipevieira]

All code issues were resolved. We're still looking into how to train the clustering algorithm to avoid incorrect clusters