Code

#20876 closed Cleanup/optimization (fixed)

Change Poll model name in tutorial to Question

Reported by: anonymous Owned by: timo
Component: Documentation Version: master
Severity: Normal Keywords: afraid-to-commit
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: yes

Description

having the project and the app both be called polls is not helpful. makes it awkward to read and understand for a new django user where exactly things are being put. how the directory / namespace structure fits together.

Attachments (0)

Change History (15)

comment:1 Changed 11 months ago by timo

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to needsinfo
  • Status changed from new to closed

I assume this is in reference to the tutorial, but the project there is called mysite so I'm not sure what this is asking for.

comment:2 Changed 11 months ago by russellm

  • Resolution needsinfo deleted
  • Status changed from closed to new
  • Triage Stage changed from Unreviewed to Accepted

I think the reporter is describing that a project called mysite, with an app called polls, with a model called Poll is asking for trouble -- it would be better to use a completely unrelated name like "survey" for the app, so you are talking about survey.Poll not polls.Poll, making it clear that it's not just a matter of pluralization.

I'm not saying survey is necessarily the best alternative name -- just pointing out that a name other than "polls" would be beneficial.

comment:3 Changed 11 months ago by koorgoo

After the Two Scoops of Django book I prefer to name apps with a plural version of the main app's model if I don't see another more obvious option.
The polls app's name seems to be clear enough.

comment:4 Changed 11 months ago by saulshanabrook

Many of my own apps share the same "problem", (i.e. artists.Artist, 'updates.Update'). I have found this to make sense when an app is really focused on a single model.

Do people consider this an antipattern that we don't want in the tutorial?

comment:5 Changed 11 months ago by timo

I'm inclined to agree with the last two comments and "won't fix", but I'll leave open for another opinion given that Russ accepted the ticket.

comment:6 Changed 11 months ago by russellm

I also agree with the previous two comments, and I make use the "plural for app, singular for model" pattern in naming my own apps - but I still think the tutorial should be changed.

We're not talking about general usage here. We're talking about a tutorial. This is the user's first experience with Django. Removing any source of confusion is a good thing. As it currently stands, we're talking about Poll, poll, and polls, and they all refer to different things -- one keystroke difference between all three. It's trivial to misread, or to become confused as to which object you're using. Grammatically (English grammar, not Python), it makes complete sense to say polls.objects.get() - so why doesn't that work? After all, I just defined a Poll object, right? I make a poll, I retrieve from a list of polls, don't I?

Once you've established the basic app/model pattern, *then* you can start introducing the idea of using pluralization in names.

If we're not going to rename the polls app, it definitely warrants a pullout box that gives an overview of the naming scheme, drawing attention to the fact that there are three distinct objects here, all with different roles.

comment:7 Changed 10 months ago by EvilDMP

  • Keywords afraid-to-commit added

I agree that this is an issue. I think that the best solution is to follow the example of the Choicemodel, so that our two models at https://docs.djangoproject.com/en/1.5/intro/tutorial01/#creating-models will look like:

class Question(models.Model):
    question_text = models.CharField(max_length=200)
    pub_date = models.DateTimeField('date published')

class Choice(models.Model):
    poll = models.ForeignKey(Question)
    choice_text = models.CharField(max_length=200)
    votes = models.IntegerField(default=0)

comment:8 Changed 10 months ago by rodolfo2488

  • Owner changed from nobody to rodolfo2488
  • Status changed from new to assigned

comment:9 Changed 10 months ago by rodolfo2488

  • Triage Stage changed from Accepted to Ready for checkin

I just made a pull request to this issue at https://github.com/django/django/pull/1574. The reason of this is that the patch file is too big to attach it in the ticket track.

comment:10 Changed 10 months ago by rodolfo2488

  • Version changed from 1.5 to master

comment:11 Changed 10 months ago by timo

  • Has patch set
  • Triage Stage changed from Ready for checkin to Accepted

Please don't mark your own patch as Ready for checkin. Someone else needs to review it and do that.

comment:12 Changed 10 months ago by timo

  • Owner changed from rodolfo2488 to timo
  • Summary changed from Polls in polls to Change Poll model name in tutorial to Question

Review in progress.

comment:13 Changed 10 months ago by rodolfo2488

Sorry, I did not see the stage of reviewing and I thought it was that.

comment:14 Changed 10 months ago by rodolfo2488

the new images are in the pull request https://github.com/django/django/pull/1574.

comment:15 Changed 10 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In d34b94b00fa817871939ea6c097621a3e4a87311:

Fixed #20876 -- Changed Poll model name in tutorial to Question

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.