Remove unnecessary dependency on pytz

[pganssle]

dateutil (already a dependency) provides a UTC object, so there's no need for a pytz dependency.

This also drops the use of datetime.utcnow in favor of the preferred datetime.now(some_tzinfo), what is being done now is effectively the equivalent of: datetime.now(UTC).replace(tzinfo=None).replace(tzinfo=UTC). utcnow is a relic from before the tz argument was added to datetime.now.

All Submissions:

• Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• Have you added an explanation of what problem you are trying to solve with this PR?
• Have you added information on what your changes do and why you chose this as your solution?
• N/A Have you written new tests for your changes? (Covered by existing tests)
• Does your submission pass tests?
• This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

[pganssle]

If you ever want to pin python-dateutil>=2.7.0, you can replace this with dateutil.tz.UTC.

[c00kiemon5ter]

I would probably use the already created instance than calling for new one (even though it is a singleton).

now = datetime.datetime.now(dateutil.tz.UTC)

[pganssle]

It's up to you, I went with tz.tzutc() because for most libraries the additional benefit of using the pre-created instance is minimal compared to the cost of pinning a dependency (which can lead to conflicts with some applications that pin to a different version). It's a bit slower to call tz.tzutc(), but I'm guessing that this function is never called in a tight loop, so 300ns doesn't make much difference either way:

In [3]: %timeit datetime.now(tz.UTC)
1.3 µs ± 7.18 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

In [4]: %timeit datetime.now(tz.tzutc())
1.65 µs ± 5.65 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

[codecov-io]

Codecov Report

Merging #517 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #517      +/-   ##
==========================================
+ Coverage   65.17%   65.21%   +0.03%     
==========================================
  Files          99       99              
  Lines       25671    25671              
==========================================
+ Hits        16731    16741      +10     
+ Misses       8940     8930      -10

Impacted Files	Coverage Δ
src/saml2/cert.py	91.19% <100%> (ø)	⬆️
src/saml2/__init__.py	86.47% <0%> (+0.19%)	⬆️
src/saml2/time_util.py	88.02% <0%> (+0.59%)	⬆️
src/saml2/validate.py	76.07% <0%> (+3.13%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3d278d...30c070c. Read the comment docs.

[c00kiemon5ter]

Hello and thanks for this. I have already dropped both pytz and dateutil dependencies in the refactor-datetime branch. Along with dropping these, the work has been centred around handling date-time objects seamlessly throughout the codebase and isolating the concepts of date-time and duration in a separate module that ensures that creation, convertion and operations on those objects are correct and in accordance to the specs (see d2de846 and issue #445 ). That branch will soon be proposed for merge and I would welcome reviews. This means that this PR will probably be dropped; but I still want to thank you for putting time into this. I am leaving this open until a final decision is made.

[pganssle]

I can't say I love the heavy use of utcnow() (which I think should be removed entirely), but I don't see you actually using it incorrectly.

I'm obviously biased as the developer of dateutil, but I personally feel that python-dateutil is a "lighter" dependency than aniso8601 simply because python-dateutil is way more likely to be on the system in the first place. It seems that you're only using aniso8601 for things also provided by dateutil (particularly if you are willing to pin to >=2.7.0 - specifically you can use dateutil.parser.isoparse, which is designed explicitly to be a strict ISO 8601 parser).

Feel free to @ me for the PR review.

[pganssle]

By the way I don't mind if you close this because the datetime refactor is coming, but it's a minor enough change that also removes an unnecessary dependency, so if it were me I'd just merge it in the meantime in case you want to cut a new release before the datetime refactor is done.

[c00kiemon5ter]

There are two things that we use from aniso8601

• the ISO 8601 parser to generate datetime.datetime objects
• the duration parser to generate datetime.timedelta objects

(We also use it to define the UTC timezone; but this was done for brevity as it was already available, and is easily replaceable by the classes provided by the standard library.)

The way the new module is written is such that it is not tied to a third party. The dependency should be easily swapped by filling in the required parsers; which should be as easy as changing the following two lines: saml2/datetime/__init__.py#L48 and saml2/datetime/duration.py#L5

---

Considering whether a library is light in the sense of it existing in a system; the trend nowadays is to use containers to deploy services. With containers the installed packages are not shared, but each container installs its own set. This means that packages are only shared between dependencies of services. But even without containers, virtualenv would be used to provide isolation.

The reason I chose aniso8601 was that

• it is focused on the standard and not generic date-time parsing
• it implements the duration part of the standard, that many other libraries do not
• it is concise and has a small codebase
• it does not use expensive regexes; in fact it uses no regexes (of course by itself this does not mean much)
• it has nice error handling; wrapping errors with custom exceptions

This does not go against dateutil which I have used a lot and will keep using in other projects (and many many thanks for making it possible for us to enjoy it 👍). In this case though, I find aniso8601 more fitting. Looking in the future, nothing is set to stone; we will always reconsider our choices and this is one of the reasons our requirements should be clearly defined and our code ready to change.

[pganssle]

Ah, some of these are good points, though just for the record

• it is focused on the standard and not generic date-time parsing
• it implements the duration part of the standard, that many other libraries do not
• it is concise and has a small codebase
• it does not use expensive regexes; in fact it uses no regexes (of course by itself this does not mean much)
• it has nice error handling; wrapping errors with custom exceptions

Specifically looking at dateutil.parser.isoparse, the ISO-specific parser also meets requirements 1, 3 and 4 (you can see the code here, which only uses 1 simple, compiled regex because it's faster than not using a regex in that case).

I did not see where you needed support for durations, but I will admit that I haven't gotten around to doing that yet. The big problem with supporting durations is that the standard includes things like "months" and "years", which are not possible to represent as a datetime.timedelta. If you need to properly support the full ISO 8601 duration standard, even aniso8601 pulls in a dateutil dependency in order to return a relativedelta (though it will do the wrong thing and give you a timedelta(30) for P1M if you don't pull in the dateutil dependency).

[c00kiemon5ter]

I am closing this but will come back to it as needed. Thanks for the discussion; I will reconsider all your points and add a relative section warning for the limitations of duration and datetime.timedelta.