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

replace utcnow #231

Merged
merged 2 commits into from
Nov 22, 2023
Merged

replace utcnow #231

merged 2 commits into from
Nov 22, 2023

Conversation

braingram
Copy link
Collaborator

@braingram braingram commented Oct 26, 2023

updates use of utcnow (deprecated in python 3.12) with an equivalent call that returns a non-timezone aware utc timestamp

Regression tests: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1023/

Showed the expected niriss failures due to crds file updates and also showed 2 failures:

  • test_nirspec_missing_msa_fail
  • test_nirspec_missing_msa_nofail

As these tests appear to sometimes fail (possibly due to test ordering or io buffering) they were re-run in another test run:
https://plwishmaster.stsci.edu:8081/blue/organizations/jenkins/RT%2FJWST-Developers-Pull-Requests/detail/JWST-Developers-Pull-Requests/1026/pipeline/199
where they both passed.

This PR removes one of the python 3.12 failures:

FAILED tests/test_util.py::test_create_history_entry - DeprecationWarning: datetime.datetime.utcnow() is deprecated and scheduled ...

see in a CI run on main:
https://github.com/spacetelescope/stdatamodels/actions/runs/6669314479/job/18126824996
but not seen in the CI run for this PR:
https://github.com/spacetelescope/stdatamodels/actions/runs/6669380674/job/18127057458?pr=231

The remaining failures are due to crds use of ast.Str: spacetelescope/crds#1000
The crds issues are addressed in: spacetelescope/crds#1007
and a test PR with the changes in this PR and crds shows passing python 3.12 unit tests in stdatamodels: #233

Checklist

  • added entry in CHANGES.rst (either in Bug Fixes or Changes to API)
  • updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f42b477) 64.67% compared to head (bb201c6) 64.62%.

❗ Current head bb201c6 differs from pull request most recent head 2611c05. Consider uploading reports for the commit 2611c05 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #231      +/-   ##
==========================================
- Coverage   64.67%   64.62%   -0.05%     
==========================================
  Files         102      102              
  Lines        5662     5660       -2     
==========================================
- Hits         3662     3658       -4     
- Misses       2000     2002       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@braingram braingram force-pushed the replace_utcnow branch 2 times, most recently from fb19682 to bb201c6 Compare October 27, 2023 15:46
@braingram braingram marked this pull request as ready for review October 30, 2023 20:28
@braingram braingram requested a review from a team as a code owner October 30, 2023 20:28
entry = HistoryEntry({
'description': description,
'time': datetime.datetime.utcnow()
'time': dt
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to not inline dt inside entry?

@hbushouse
Copy link
Collaborator

@braingram Could you rebase with latest masters and run the regtests again?

@braingram
Copy link
Collaborator Author

There's a similar PR for jwst: spacetelescope/jwst#8051
I was going to leave that as draft until the effort to support python 3.12 was started but should I update that jwst PR also (and open it for review)?

@hbushouse
Copy link
Collaborator

There's a similar PR for jwst: spacetelescope/jwst#8051 I was going to leave that as draft until the effort to support python 3.12 was started but should I update that jwst PR also (and open it for review)?

I don't see the harm in future proofing the code now, even before we're ready to allow py 3.12. That way it's ready to go, and any tests we run against 3.12 won't have this issue.

@braingram
Copy link
Collaborator Author

@braingram
Copy link
Collaborator Author

Regression tests passed with no errors.

@hbushouse hbushouse added this to the 1.8.4 milestone Nov 22, 2023
@hbushouse hbushouse merged commit 0ed03c2 into spacetelescope:master Nov 22, 2023
19 of 20 checks passed
@braingram braingram deleted the replace_utcnow branch November 22, 2023 20:58
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