Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pep8 for schedules app #23

Merged
merged 6 commits into from
Mar 7, 2016
Merged

pep8 for schedules app #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
28e3aca
pep8 for schedules app, minor code changes for cleaner code
stefan-walluhn Mar 1, 2016
f16c522
some more pep8 changes for schedules app
stefan-walluhn Mar 1, 2016
5baccd5
fix unicode built-in for Schedule model
stefan-walluhn Mar 1, 2016
69e7586
Merge branch 'master' of https://github.com/iago1460/django-radio int…
stefan-walluhn Mar 7, 2016
458dffb
back to 120 char line length
stefan-walluhn Mar 7, 2016
108ee6f
fix redundant brackets for long ifs
stefan-walluhn Mar 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#django collectstatic files
radio/media/_versions/

# Byte-compiled / optimized / DLL files
__pycache__/
.DS_Store
Expand All @@ -23,6 +26,12 @@ build/
/django_radio.egg-info/
sitepackages/

#virtualenv
bin/
include/
lib/
local/

# Installer logs
pip-log.txt
pip-delete-this-directory.txt
Expand Down
10 changes: 4 additions & 6 deletions radio/apps/schedules/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,6 @@ def copy_ScheduleBoard(self, request, queryset):
copy_ScheduleBoard.short_description = _("Make a Copy of calendar")


admin.site.register(ScheduleBoard, ScheduleBoardAdmin)


class FullcalendarAdmin(admin.ModelAdmin):
def schedule_detail(self, request):
return HttpResponseRedirect(reverse("dashboard:schedule_editor"))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bunch admin registration at the end of module to not spread registration logic across the code

Expand All @@ -97,8 +94,8 @@ def get_urls(self):
'app_name': self.model._meta.app_label,
'model_name': self.model._meta.model_name,
}
custom_urls = patterns('',
url(
custom_urls = patterns(
'', url(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was mangled by the pep8 tool (continuation line under-indented for visual indent)

r'^$',
self.admin_site.admin_view(self.schedule_detail),
{},
Expand All @@ -111,11 +108,12 @@ def get_urls(self):
return custom_urls + urls

def response_change(self, request, obj):
msg = _('%(obj)s was changed successfully.') % {'obj': force_unicode(obj)}
msg = _('{obj} was changed successfully.'.format(obj=force_unicode(obj)))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer format() over "%" syntax, but this is just personal taste. Feel free to mangle this change.

if '_continue' in request.POST:
return HttpResponseRedirect(request.path)
else:
return HttpResponseRedirect("../../")


admin.site.register(ScheduleBoard, ScheduleBoardAdmin)
admin.site.register(Schedule, FullcalendarAdmin)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bunch admin registration at the end of module

61 changes: 51 additions & 10 deletions radio/apps/schedules/migrations/0001_initial.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,31 @@ class Migration(migrations.Migration):
migrations.CreateModel(
name='Schedule',
fields=[
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
('day', models.IntegerField(choices=[(0, 'Monday'), (1, 'Tuesday'), (2, 'Wednesday'), (3, 'Thursday'), (4, 'Friday'), (5, 'Saturday'), (6, 'Sunday')])),
('id', models.AutoField(
verbose_name='ID',
serialize=False,
auto_created=True,
primary_key=True)),
('day', models.IntegerField(
choices=[
(0, 'Monday'),
(1, 'Tuesday'),
(2, 'Wednesday'),
(3, 'Thursday'),
(4, 'Friday'),
(5, 'Saturday'),
(6, 'Sunday')])),
('start_hour', models.TimeField(verbose_name='start time')),
('type', models.CharField(max_length=1, verbose_name='type', choices=[(b'L', 'live'), (b'B', 'broadcast'), (b'S', 'broadcast syndication')])),
('programme', models.ForeignKey(verbose_name='programme', to='programmes.Programme')),
('type', models.CharField(
max_length=1,
verbose_name='type',
choices=[
(b'L', 'live'),
(b'B', 'broadcast'),
(b'S', 'broadcast syndication')])),
('programme', models.ForeignKey(
verbose_name='programme',
to='programmes.Programme')),
],
options={
'verbose_name': 'schedule',
Expand All @@ -30,10 +50,23 @@ class Migration(migrations.Migration):
migrations.CreateModel(
name='ScheduleBoard',
fields=[
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
('name', models.CharField(unique=True, max_length=255, verbose_name='name')),
('start_date', models.DateField(null=True, verbose_name='start date', blank=True)),
('end_date', models.DateField(null=True, verbose_name='end date', blank=True)),
('id', models.AutoField(
verbose_name='ID',
serialize=False,
auto_created=True,
primary_key=True)),
('name', models.CharField(
unique=True,
max_length=255,
verbose_name='name')),
('start_date', models.DateField(
blank=True,
null=True,
verbose_name='start date')),
('end_date', models.DateField(
blank=True,
null=True,
verbose_name='end date')),
],
options={
'verbose_name': 'schedule board',
Expand All @@ -44,13 +77,21 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name='schedule',
name='schedule_board',
field=models.ForeignKey(verbose_name='schedule board', to='schedules.ScheduleBoard'),
field=models.ForeignKey(
verbose_name='schedule board',
to='schedules.ScheduleBoard'),
preserve_default=True,
),
migrations.AddField(
model_name='schedule',
name='source',
field=models.ForeignKey(on_delete=django.db.models.deletion.SET_NULL, blank=True, to='schedules.Schedule', help_text='It is used when is a broadcast.', null=True, verbose_name='source'),
field=models.ForeignKey(
on_delete=django.db.models.deletion.SET_NULL,
blank=True,
null=True,
to='schedules.Schedule',
help_text='It is used when is a broadcast.',
verbose_name='source'),
preserve_default=True,
),
]
100 changes: 42 additions & 58 deletions radio/apps/schedules/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

from apps.programmes.models import Programme, Episode


emission_type = (
("L", _("live")),
("B", _("broadcast")),
Expand Down Expand Up @@ -61,22 +62,21 @@ def clean(self):
if self.start_date > self.end_date:
raise ValidationError(_('end date must be greater than or equal to start date.'))
# check date collision
qs = ScheduleBoard.objects.filter(
start_date__lte=self.end_date, end_date__isnull=True
) | ScheduleBoard.objects.filter(
start_date__lte=self.end_date, end_date__gte=self.start_date
qs = (
ScheduleBoard.objects.filter(start_date__lte=self.end_date, end_date__isnull=True) |
ScheduleBoard.objects.filter(start_date__lte=self.end_date, end_date__gte=self.start_date)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable. The important part is to apply multiple filters, so the code should not spread this across multiple indentation levels, but should reflect this by intuitive reading.

)

if self.pk is not None:
qs = qs.exclude(pk=self.pk)
if qs.exists():
raise ValidationError(_('there is another object between this dates.'))

else:
# start_date != None and end_date == None only one can exist
qs = ScheduleBoard.objects.filter(
start_date__isnull=False, end_date__isnull=True
) | ScheduleBoard.objects.filter(
end_date__gte=self.start_date
qs = (
ScheduleBoard.objects.filter(start_date__isnull=False, end_date__isnull=True) |
ScheduleBoard.objects.filter(end_date__gte=self.start_date)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable. The important part is to apply multiple filters, so the code should not spread this across multiple indentation levels, but should reflect this by intuitive reading.

)
if self.pk is not None:
qs = qs.exclude(pk=self.pk)
Expand All @@ -86,9 +86,6 @@ def clean(self):

elif self.end_date:
raise ValidationError(_('start date cannot be null if end date exists'))
else:
# both None = disable
pass

def save(self, *args, **kwargs):
# rearrange episodes
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has no effect at all.

Expand All @@ -104,11 +101,10 @@ def save(self, *args, **kwargs):

@classmethod
def get_current(cls, dt):
schedule_board = cls.objects.filter(
start_date__lte=dt, end_date__isnull=True
).order_by('-start_date') | cls.objects.filter(
start_date__lte=dt, end_date__gte=dt
).order_by('-start_date')
schedule_board = (
cls.objects.filter(start_date__lte=dt, end_date__isnull=True).order_by('-start_date') |
cls.objects.filter(start_date__lte=dt, end_date__gte=dt).order_by('-start_date')
)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable. The important part is to apply multiple filters, so the code should not spread this across multiple indentation levels, but should reflect this by intuitive reading.

return schedule_board.first()

class Meta:
Expand Down Expand Up @@ -149,24 +145,23 @@ def __get_rrule(self):
# Due to rrule we need to add 1 day
end_date = end_date + datetime.timedelta(days=1)
return rrule.rrule(
rrule.WEEKLY, byweekday=[self.day], dtstart=datetime.datetime.combine(
start_date, self.start_hour
), until=end_date
)
rrule.WEEKLY, byweekday=[self.day],
dtstart=datetime.datetime.combine(start_date, self.start_hour),
until=end_date)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable, do not split datetime.combine function call across multiple lines resulting in spread rrule params where we are not forced to do so.

else:
end_date = self.schedule_board.end_date
if end_date:
# Due to rrule we need to add 1 day
end_date = end_date + datetime.timedelta(days=1)
return rrule.rrule(
rrule.WEEKLY, byweekday=[self.day], dtstart=datetime.datetime.combine(
start_date, self.start_hour
), until=end_date
rrule.WEEKLY, byweekday=[self.day],
dtstart=datetime.datetime.combine(start_date, self.start_hour),
until=end_date
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable, do not split datetime.combine function call across multiple lines resulting in spread rrule params where we are not forced to do so.

)
else:
return rrule.rrule(
rrule.WEEKLY, byweekday=[self.day], dtstart=datetime.datetime.combine(
start_date, self.start_hour)
rrule.WEEKLY, byweekday=[self.day],
dtstart=datetime.datetime.combine(start_date, self.start_hour)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable, do not split datetime.combine function call across multiple lines resulting in spread rrule params where we are not forced to do so.

)

def dates_between(self, after, before):
Expand Down Expand Up @@ -209,16 +204,11 @@ def clean(self):
start_date = date
end_date = start_date + schedule.runtime()
raise ValidationError(
_(
'This settings collides with: %(name)s [%(start_date)s %(start_day)s/%(start_month)s/%(start_year)s - %(end_date)s %(end_day)s/%(end_month)s/%(end_year)s ]'
) % {
'name': schedule.programme.name, 'start_date': start_date.strftime("%H:%M"),
'start_day': start_date.strftime("%d"),
'start_month': start_date.strftime("%m"),
'start_year': start_date.strftime("%Y"),
'end_date': end_date.strftime("%H:%M"), 'end_day': end_date.strftime("%d"),
'end_month': end_date.strftime("%m"), 'end_year': end_date.strftime("%Y"),
}
_('This settings collides with: {name} [{start_date} - {end_date}]').format(
name=schedule.programme.name,
start_date=start_date.strftime("%H:%M %d/%m/%Y"),
end_date=end_date.strftime("%H:%M %d/%m/%Y")
)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datetime.strftime() is already dedicated to perform date string formating. We do not need to increase complexity to format the date string a second time on multiple levels.

)
index += 1

Expand All @@ -230,23 +220,22 @@ def save(self, *args, **kwargs):

@classmethod
def between(cls, after, before, exclude=None, live=False, schedule_board=None):
list_schedules = cls.objects.filter(
programme__start_date__lte=before, programme__end_date__isnull=True
) | cls.objects.filter(
programme__start_date__lte=before, programme__end_date__gte=after
list_schedules = (
cls.objects.filter(programme__start_date__lte=before, programme__end_date__isnull=True) |
cls.objects.filter(programme__start_date__lte=before, programme__end_date__gte=after)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable. The important part is to apply multiple filters, so the code should not spread this across multiple indentation levels, but should reflect this by intuitive reading.

)
if live:
list_schedules = list_schedules.filter(type='L')
if schedule_board:
list_schedules = list_schedules.filter(schedule_board=schedule_board)
else:
list_schedules = list_schedules.filter(
schedule_board__start_date__lte=before, schedule_board__end_date__isnull=True
) | list_schedules.filter(
schedule_board__start_date__lte=before, schedule_board__end_date__gte=after
list_schedules = (
list_schedules.filter(schedule_board__start_date__lte=before, schedule_board__end_date__isnull=True) |
list_schedules.filter(schedule_board__start_date__lte=before, schedule_board__end_date__gte=after)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable. The important part is to apply multiple filters, so the code should not spread this across multiple indentation levels, but should reflect this by intuitive reading.

)
if exclude:
list_schedules = list_schedules.exclude(id=exclude.id)

list_schedules = list_schedules.order_by('-programme__start_date').select_related(
'programme', 'schedule_board', 'source__programme', 'source__schedule_board'
)
Expand All @@ -262,18 +251,16 @@ def between(cls, after, before, exclude=None, live=False, schedule_board=None):

@classmethod
def schedule(cls, dt, exclude=None):
list_schedules = cls.objects.filter(
programme__start_date__lte=dt, programme__end_date__isnull=True
) | cls.objects.filter(
programme__start_date__lte=dt, programme__end_date__gte=dt
list_schedules = (
cls.objects.filter(programme__start_date__lte=dt, programme__end_date__isnull=True) |
cls.objects.filter(programme__start_date__lte=dt, programme__end_date__gte=dt)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable. The important part is to apply multiple filters, so the code should not spread this across multiple indentation levels, but should reflect this by intuitive reading.

)
if exclude:
list_schedules = list_schedules.exclude(id=exclude.id)
list_schedules = list_schedules.select_related('programme', 'schedule_board')
earlier_date = None
earlier_schedule = None
for schedule in list_schedules:
# if schedule != exclude:
date = schedule.date_before(dt)
if date and (earlier_date is None or date > earlier_date):
earlier_date = date
Expand All @@ -285,20 +272,17 @@ def schedule(cls, dt, exclude=None):
@classmethod
def get_next_date(cls, programme, after):
list_schedules = cls.objects.filter(programme=programme, type='L')
list_schedules = list_schedules.filter(
programme__end_date__isnull=True
) | list_schedules.filter(
programme__end_date__gte=after
list_schedules = (
list_schedules.filter(programme__end_date__isnull=True) |
list_schedules.filter(programme__end_date__gte=after)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable. The important part is to apply multiple filters, so the code should not spread this across multiple indentation levels, but should reflect this by intuitive reading.

)
list_schedules = list_schedules.filter(
schedule_board__start_date__isnull=False, schedule_board__end_date__isnull=True
) | list_schedules.filter(
schedule_board__end_date__gte=after
list_schedules = (
list_schedules.filter(schedule_board__start_date__isnull=False, schedule_board__end_date__isnull=True) |
list_schedules.filter(schedule_board__end_date__gte=after)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make code more readable. The important part is to apply multiple filters, so the code should not spread this across multiple indentation levels, but should reflect this by intuitive reading.

)
closer_date = None
closer_schedule = None
for schedule in list_schedules:
# if schedule != exclude:
date = schedule.date_after(after)
if date and (closer_date is None or date < closer_date):
closer_date = date
Expand All @@ -308,7 +292,7 @@ def get_next_date(cls, programme, after):
return closer_schedule, closer_date

def __unicode__(self):
return self.get_day_display() + ' - ' + self.start_hour.strftime('%H:%M')
return ' - '.join([self.get_day_display(), self.start_hour.strftime('%H:%M')])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is classical string joining, so no need for hacky string arithmetic.


class Meta:
verbose_name = _('schedule')
Expand Down
Loading