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.

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

Subtract dates not datetimes #3219

Merged
merged 4 commits into from
Nov 7, 2023
Merged

Subtract dates not datetimes #3219

merged 4 commits into from
Nov 7, 2023

Conversation

dgboss
Copy link
Collaborator

@dgboss dgboss commented Nov 6, 2023

The bug manifested because of the time change. When subtracting datetime objects to get a timedelta, you get an extra day if your dates span a time change. Switching to subtracting dates avoids this problem.

Test Links:

Landing Page
MoreCast 2.0
Percentile Calculator
MoreCast
C-Haines
FireBat
FireBat bookmark
Auto Spatial Advisory (ASA)
HFI Calculator

Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Merging #3219 (c8d1304) into main (41334be) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3219      +/-   ##
==========================================
+ Coverage   82.05%   82.08%   +0.03%     
==========================================
  Files         274      274              
  Lines        9301     9301              
  Branches      413      413              
==========================================
+ Hits         7632     7635       +3     
+ Misses       1557     1556       -1     
+ Partials      112      110       -2     
Files Coverage Δ
api/app/utils/time.py 88.88% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@conbrad
Copy link
Collaborator

conbrad commented Nov 7, 2023

When subtracting datetime objects to get a timedelta, you get an extra day if your dates span a time change

Think it'd be worth added a test for this case? 2 datetimes, one pst, one pdt, fails before your change, passes after your change?

Copy link

sonarcloud bot commented Nov 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@dgboss
Copy link
Collaborator Author

dgboss commented Nov 7, 2023

When subtracting datetime objects to get a timedelta, you get an extra day if your dates span a time change

Think it'd be worth added a test for this case? 2 datetimes, one pst, one pdt, fails before your change, passes after your change?

Ha, started on the tests before I saw this! Did one for the time change in both directions.

@dgboss dgboss temporarily deployed to production November 7, 2023 16:57 Inactive
@dgboss dgboss merged commit d1d8460 into main Nov 7, 2023
28 checks passed
@dgboss dgboss deleted the bug/page-crash/3215 branch November 7, 2023 17:24
vanislekahuna pushed a commit to vanislekahuna/wps that referenced this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants