TabbycatDebate / tabbycat

Debating tournament tabulation software for British Parliamentary and a variety of two-team parliamentary formats
https://tabbycat.readthedocs.io/
GNU Affero General Public License v3.0
241 stars 806 forks source link

Add user-friendly data import function #24

Closed czlee closed 9 years ago

czlee commented 10 years ago

To set up a tournament without needing to write a Python script (which is the status quo).

Possible ways of doing this

philipbelesky commented 10 years ago

One thing I noticed while working on this is that the model for Teams is very odd in terms of how it represents the team names, and all the flags with showing/hiding institution names and team numbers. This makes importing data very annoying, especially for smaller tournaments and tournaments where the team's institution is not very relevant.

I would heavily prefer each team is represented/importable based on a name (a string which wouldn't have to be unique) and Institution. Adding on institutions/prefixes and team numbers can then be done through views/templates/utility functions without having to be represented in the model. This will also allow much more flexibility in customising how condensed these abbreviations and the like should be.

czlee commented 10 years ago

There are a couple of ideas behind the current implementation:

  1. A team name based on its institution should be represented as such, partly for fidelity and partly so it can be changed easily. For example, if the institution code for "Victoria" changes to "Victoria Wellington", so should all of its teams' names.
  2. Sometimes it is appropriate to call a team by its institution's whole name, followed by its number, e.g., "National University of Singapore 2".

But that said, the current implementation probably has something of a hangover from the original implementation, which allowed for no relationship between institution name and team name. Also, when I changed it, I wanted to allow for tournaments where teams were named not numbered (e.g. Thropy), or where institutions aren't as relevant, which possibly led to the proliferation of irritating flags.

I'm a little confused about what you mean though. Is the idea to move the current team-name permutations functionality out of the models and into views? I'm down for that. We'd just need a way to distinguish between "Auckland Charmander" (institution code part of team name) and just "Charmander" from Auckland - but one could argue this is a property of the tournament, not the team.

philipbelesky commented 10 years ago

Yea, its a tricky design problem, both on the front end and backend. The current per-team toggle for "show institutional prefix" is good, meaning we can specify these things on a case by case basis. In my current branch I've added a short_name method that uses self.institution.code + name rather than long_name's self.institution.name + name. This is probably the best default in that it ensures uniqueness but is also compact.

Note that I've also added institution columns to many of the results page's (ie speaker team standings), partly as a bit of extra information, and partly as a fallback for when team's aren't using institutional prefixes but institutions are still relevant (perhaps Thropy?). These columns can be globally toggled off in the Config page (ie for a club's internal competition).

Keeping the naming functions in the model is probably fine for now. I forgot what the problem was for importing, possibly it was a result of using parts of the import script that were written before a model change. If you want to have a look my script is in _debate/management/commands/importtournament.py

czlee commented 10 years ago

I can imagine it being an issue for imports, mainly because we'd have to educate users about how to configure the spreadsheet so that the system knows the difference. Like, we could include a boolean field or something in the spreadsheet, but it'd be annoying to have to get your head around.

What does Team.name do now? Is it ever useful (being distinct from Team.reference)? Or should we make Team.name and Team.short_name do the same thing?

philipbelesky commented 10 years ago

Yea, Team.name is redundant. I think it would be good to keep the methods as Team.short_name and Team.long_name if just to emphasise that there are two different styles of name that both are not canonical to the reference or initially-imported name.

We could also add a global toggle for "show institutional shortcode" that could be overridden if each team's "show institution" is set, so that the setting wouldn't need to be in an imported spreadsheet. Smaller tournaments are the ones that probably need the setting to be done on a case-by-case basis, so the manual editing shouldn't be too troublesome.

czlee commented 10 years ago

Okay, so so far we're at:

Remaining question: What do we want to do about the __unicode__ function? Currently a lot of the templates rely on it, which we should probably change (or decide that __unicode__ == short_name).

philipbelesky commented 10 years ago

short_name is probably the better form to use? Although we might have to add a check into short_name to check if the Institution actually has a Institution.code value (its not a required field IIRC?)

czlee commented 10 years ago

Oh, but reference and short_name aren't the same thing, at least not at the moment:

I think Institution.code is required. Code:

class Institution(models.Model):
    tournament = models.ForeignKey(Tournament)
    code = models.CharField(max_length=20)
    name = models.CharField(max_length=100)
    abbreviation = models.CharField(max_length=8, default="")
philipbelesky commented 10 years ago

reference and short_name being different is as it should be? Sounds good to go ahead with removing Team.name

czlee commented 10 years ago

How about __unicode__? I'm inclined to make it the same as short_name, mostly because in most contexts they will actually match, and long_name is sort of used only in a handful of contexts (like, say, break round announcements and the publishable team tab).

Or we could make a rule that we don't rely on __unicode__, but we'd have to be pretty careful to update all the templates accordingly.

philipbelesky commented 10 years ago

Isn't unicode a method that most models are meant to have?

But yea, setting it to short_name seems best given that seems to be the most common visual representation in use

On Thu, May 22, 2014 at 3:15 AM, Chuan-Zheng Lee notifications@github.com wrote:

How about __unicode__? I'm inclined to make it the same as short_name, mostly because in most contexts they will actually match, and long_name is sort of used only in a handful of contexts (like, say, break round announcements and the publishable team tab).

Or we could make a rule that we don't rely on __unicode__, but we'd have to be pretty careful to update all the templates accordingly.

Reply to this email directly or view it on GitHub: https://github.com/czlee/tabbycat/issues/24#issuecomment-43787035

czlee commented 10 years ago

Oh, I mean, we could say by convention that the __unicode__ representation is never seen on any interface and is used only for debugging, and then make it a more verbose description or something. But I'm inclined to go with short_name.

philipbelesky commented 10 years ago

Yea, the long_name is barely more verbose/semantic than the short_name so best keep things consistent.

czlee commented 9 years ago

Obsoleted by #157