team.py:67 and its associated test (test_team.py:40) are built on the assumption that datetime.strftime('%Y') will always return the year-based season identifier to use when building URLs. However prior to January 1 of each year this would return the season prior to the "current" one.
The fix for team.py would be to prevent this logic from adding any URL parameters when the season argument isn't specified, since team.php on KenPom.com treats the absence of this parameter as a request for whatever the current year happens to be.
For the test, I like that we have coverage on the exceptions (currentYear+1) but we may need to rethink the strategy. We need to dig into why this wasn't failing since currentYear + 1 should resolve and not throw any exception.
A "nice to have" in the PR that fixes this would be to rename currentYear to currentSeason, but this is quite the nitpick and wouldn't be a deal breaker for getting a merge.
team.py:67
and its associated test (test_team.py:40
) are built on the assumption thatdatetime.strftime('%Y')
will always return the year-based season identifier to use when building URLs. However prior to January 1 of each year this would return the season prior to the "current" one.The fix for
team.py
would be to prevent this logic from adding any URL parameters when theseason
argument isn't specified, sinceteam.php
on KenPom.com treats the absence of this parameter as a request for whatever the current year happens to be.For the test, I like that we have coverage on the exceptions (
currentYear+1
) but we may need to rethink the strategy. We need to dig into why this wasn't failing sincecurrentYear + 1
should resolve and not throw any exception.A "nice to have" in the PR that fixes this would be to rename
currentYear
tocurrentSeason
, but this is quite the nitpick and wouldn't be a deal breaker for getting a merge.