Skip to content

Conversation

@youknowone
Copy link
Contributor

@youknowone youknowone commented Jan 18, 2026

The _datetime dependency of test_sys is only required for @cpython_only decorated test.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

self.assertEqual(sys.getsizeof(True, -1), size('') + self.longdigit)

def test_objecttypes(self):
import _datetime
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to import it just before it is used, like collections.

And since it is optional, skip the corresponding test if the import fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Fixed it

Copy link
Member

Choose a reason for hiding this comment

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

And since it is optional, skip the corresponding test if the import fails.

_datetime is now a built-in module, it's not really optional.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't it just an implementation detail? It was made builtin to solve a particular technical issue which can be solved in other way.

@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jan 20, 2026
check(x, size('5Pi'))
# PyCapsule
import _datetime
check(_datetime.datetime_CAPI, size('6P'))
Copy link
Member

Choose a reason for hiding this comment

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

As @serhiy-storchaka requested, this line should only be run if _datetime is available. Something like:

try:
    import _datetime
except ImportError:
    pass
else:
    check(_datetime.datetime_CAPI, size('6P'))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants