FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-120782: Update internal type cache when reloading datetime by neonene · Pull Request #120829 · python/cpython · GitHub

/ cpython Public

gh-120782: Update internal type cache when reloading datetime #120829

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.

Sign up for GitHub

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

Merged
kumaraditya303 merged 3 commits into from
Jun 21, 2024
Conversation 11 Commits 3 Checks 0 Files changed
Merged

gh-120782: Update internal type cache when reloading datetime #120829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9bdc48
add a test
neonene Jun 21, 2024
7c10284
invalidate type caches on reload
neonene Jun 21, 2024
285c3b6
📜🤖 Added by blurb_it.
blurb-it[bot] Jun 21, 2024
File filter

Filter by extension

Filter by extension .c  (1) .py  (1) .rst  (1) All 3 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
17 changes: 17 additions & 0 deletions Lib/test/datetimetester.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -6870,6 +6870,23 @@ def pickle_fake_date(datetime_) -> Type[FakeDate]:
""")
script_helper.assert_python_ok('-c', script)

def test_update_type_cache(self):
# gh-120782
script = textwrap.dedent("""
import sys
for i in range(5):
import _datetime
_datetime.date.max > _datetime.date.min
_datetime.time.max > _datetime.time.min
_datetime.datetime.max > _datetime.datetime.min
_datetime.timedelta.max > _datetime.timedelta.min
isinstance(_datetime.timezone.min, _datetime.tzinfo)

JelleZijlstra Jun 21, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should this have assert?

neonene Jun 21, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality 
    assert isinstance(_datetime.timezone.min, _datetime.tzinfo)
           ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

    assert not isinstance(_datetime.timezone.min, _datetime.tzinfo)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

Both fail, so this checks if it crashes or not. I agree this is not a good example.

ericsnowcurrently Jun 21, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Given that the code looks a bit unusual, it would be worth adding a brief comment explaining that.

(Sorry I didn't notice this during my earlier review. Good catch, @JelleZijlstra.)

JelleZijlstra Jun 21, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Wouldn't it make sense to have these asserts actually check that the values are of the type they should be? The bug manifested itself as these values appearing to be objects of various random types.

JelleZijlstra Jun 21, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You could also assert that timezone.__dict__["min"] is timezone.min.
isinstance(_datetime.timezone.utc, _datetime.tzinfo)
isinstance(_datetime.timezone.max, _datetime.tzinfo)
del sys.modules['_datetime']
""")
script_helper.assert_python_ok('-c', script)


def load_tests(loader, standard_tests, pattern):
standard_tests.addTest(ZoneInfoCompleteTest())
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2024-06-21-12-00-16.gh-issue-120782.LOE8tj.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix wrong references of the :mod:`datetime` types after reloading the module.
6 changes: 6 additions & 0 deletions Modules/_datetimemodule.c
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -7296,6 +7296,12 @@ _datetime_exec(PyObject *module)
static_assert(DI100Y == 25 * DI4Y - 1, "DI100Y");
assert(DI100Y == days_before_year(100+1));

if (reloading) {
for (size_t i = 0; i < Py_ARRAY_LENGTH(capi_types); i++) {
PyType_Modified(capi_types[i]);
}
}

if (set_current_module(interp, module) < 0) {
goto error;
}
Expand Down
Back | FazBrowse Home | New Git URL