Weeks unfortunately don't always start on a monday. In some locales it's sundays.

@bomjacob Does this actually matter all that much? I know this might come off as rude, but I don't see why this matters. It's a really tiny thing.

And as far as I know, it would require too much code to be able to do an change to where Sunday is day 0 in certain locales, as we don't even know how many locales have Sunday as the first day of the week.

Well I'm not sure, but I'd say that the entirety of North America is a pretty big demographic...
Question is if it even matters... as long as the code below correctly knows that a monday is a monday (and not necesarily a 0) then everything is fine... I'm not entirely sure that python is that smart though :/

@bomjacob I've already done my research on this, Python2 and Python 3 both return the .weekday() from 0-6 where monday is 0 and sunday is 6.

If that's the case across all locales (that is, python doesn't automatically change that weekday depending on where it's run) then it shouldn't matter and this should be fine...

According to the official Datetime docs that is the case, so I think we're good on the mapping of the days.

Why not use .total_seconds() here?

This debug would wrongly say that it's running a job even though it's not, if it's on an excluded day.

I've fixed this.

I'm using a separate get_days function here, because if I put it in the class, it won't be filtered out by the list comprehension, and I won't be able to easily get the values from the attributes defined in the class.

Please split this into multiple test that only test one thing each.
E.g. have one that tests if seconds (remember to test floats too btw) and another test which tests if datetime objects work.

I cannot find a reliable way to test the float, one time it succeeds, one time it fails. Do you know a way in which I can reliably test floats? I know for certain that floats work, is not testing them an option, because if the test starts on the wrong millisecond and rounds the wrong way, the test will fail.

Enums are a new feature that appeared in 3.4. Note that this library also supports python 2.7.
While they have been backported and are available on pypi, I would rather not add a whole dependency for something that is used so very little.

@bomjacob I don't think one extra dependency will hurt people. And Enums are not used so "very little". And there's no way to track how often Enums are being used either. But I know that they're used more than your "so very little" stat.

Granted, we use enums elsewhere in the project... but a enum in python could just as well be a class with some attributes.... There's a reason it wasn't added until 3.4... it's pretty much unnecessary. And of cause if there was a library that would make our library easier to maintain, easier to read or easier to develop on, we wouldn't hesitate to add it as a dependency - but Enums simply aren't that.

@bomjacob I'd personally like to keep the enum library, it's supported and a very clean way of doing it, it isn't a very big dependency, and could be used more throughout the project as it grows.

An alternative would look like:

>>> def enum(**named_values):
...     return type('Enum', (), named_values)
...
>>> Days = enum(mon=0, tue=1, wed=2, thu=3, fri=4, sat=5, sun=6)
>>> Days.mon
0
>>> class Days(object):
...     def __init__(self, day):
...         if day not in (Days.mon , Days.tue, Days.wed, Days.thu, Days.fri, Days.sat, Days.sun):
...             raise ValueError('gender not valid')
...         self.day = day
...

I don't like this pattern because it relies upon the integers too much and has way more overhead.
In my original example the Enum doesn't relie on the integers too much, only to check if one of the values of the Days which were supplied with the Job is today. The Enum subclass returns a special Enumeration object instead of the literal integer value. This is a lot nicer for error handling because people should use the supplied Days Enum subclass. If you don't do this, people could just pass in integers from 0-6 and it'd accept. As far as I know, we can't check if a value comes from a certain class if we do this with a plain new-style class. With an Enum we can check if it comes from the class it should come from and enforce people to use the Days Enum subclass.

Why do you want to prevent users from using integers? It's not really very python like to require inputs to be of a special type if you don't have to. My recommendation would be as follows:
Make the Days a simple class with the weekdays as class variables (attributes), this is what we use for enums elsewhere in the project and it's worked fine (if you believe that it absolutely wouldn't work here, please do correct me).
Just raise a ValueError if 0 < day > 7 inside ``Job's init` or `JobQueue`'s `put`.

In the future we might upgrade to use Enums everywhere, but for now it's simply not necessary.

Code is more often read than written. Preventing users from using plain integers makes it clearer what someone's trying to accomplish when looking over the code.

For example:

import datetime

test_job = Job(callback, datetime.time(hour=3), repeat=True, days=(0, 4, 6))

If I make my bot open source and someone reads this, it's very unclear what the original developer tries to do without first reading the documentation of this repo. It's also very confusing if you have different locals (like you pointed out) and you assume that 0 is sunday instead of monday. Of course, we would like to make the developer use our Days class. But he/she isn't enforced to do it, and I think we should assume people wouldn't if they don't really have to.

This code:

import datetime

from telegram.ext.jobqueue import Days

test_job = Job(callback, datetime.time(hour=3), repeat=True, days=(Days.mon, Days.fri, Days.sun))

This code is a little longer, but it's entirely self-documenting! A developer who is unknown with the codebase will instantly know how it works. The syntax is clear, the functionality is clear, and we're enforcing them to use the Enum from the project instead of integers like we saw in the first example. I think this is more Pythonic, because "there should be one-- and preferably only one --obvious way to do it", except for string formatting.

I agree to use enum because Python 2 is going to drop its support soon. Python 3 is the new standard.

Please see @tsnoam comment below. Bottom line is that for now, don't use Enums unless they're part of future (which I don't think they are).

Please just write the msg directly in the exception, it simply looks a bit cleaner.

Everywhere or only this instance?

Everywhere preferably :)

What is the max line length for the project? I use 84 as my personal standard, so when a message is too long, I use a variable.

@voider1 Max line length is 99 characters.

I know what this is supposed to do, but it's still unreadable. I think the best solution would be to do something like we did in MessageEntity. Other ideas?

Since we switched to a class with constants, the correct naming convention would be UPPER_CASE if I'm not mistaken.

This method seems to be redundant, especially since this looks like a purely internal method and you already check isinstance(var, datetime.timedelta)

Please update the docstring so it mentions that next_t can also be of type timedelta

Debug print

According to my earlier comment, this would change to days=Days.ALL_DAYS (or maybe even days=Days.EVERY_DAY? Your choice ^^)

I believe you switched to timedelta instead of time, please update this docstring to reflect that change.

If someone would, for whatever reason, put a day twice (like Job(..., days=(Days.mon, Days.mon))) this would run the job twice on those days. I'm not sure this is a bug or a feature, but it could be easily changed by changing the type of Job.days from tuple to set. Your choice.