Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
gh-119127: functools.partial placeholders #119827
gh-119127: functools.partial placeholders #119827
Changes from 70 commits
ee7333c
8bcc462
c67c9b4
680d900
9591ff5
067e938
8af20b3
607a0b1
f55801e
5894145
3722e07
a79c2af
12aaa72
92c767b
496a9d2
38d9c11
707b957
14b38ca
32bca19
8576493
a3fd2d6
0852993
6fea348
caec6e8
115b8c5
3f5f00b
202c929
2c16d38
400ff55
8ccc38f
e7c82c7
c9b7ef3
e59d711
7bfc591
7957a97
8aaee6a
fe8e0ad
00dd80e
d352cfa
9038ed5
49b8c71
bc1fdbd
3067221
1185510
266b4fa
dd58a12
5971fbb
9033650
d31e5d1
a3d39b0
9e4c5df
16f12f8
82dd600
f9cb653
d255524
404044e
800217b
38ee450
11f47db
3c872bd
fd16189
a6c6ef2
1c8d73e
a8bd3ae
70e47ed
2eacf5e
f78d8d3
0a8640e
6e3d282
66c305d
14bf68c
ee642d5
8d6c28e
8744bcb
b896470
4881ae6
c3ad7d9
5e5d484
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is overcomplication. The user has no reasons to create an instance of private class
_PlaceholderType
. And if they need, they can do it anyway by usingobject.__new__(_PlaceholderType)
.If you want to add some guards here, just make
__new__
always raising an exception and create the single instance asobject.__new__(_PlaceholderType)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was to mimic C implementation, which has the behaviour of
None
.I think the question is: "Is it a good practice for a non-trivial sentinel to be singleton, i.e.
type(None)() is None
.If yes and this sentinel is considered non-trivial, then this is as good as it can get for now and protection issues can be sorted out together with further developments in this area.
If no, then this needs to be changed for both C and Python.
@rhettinger has suggested this initially and I like this behaviour (and adapted it to my own sentinels). It would be good if you together could come to agreement before I make any further changes here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not required for its main function, and this complicates both implementations. It is better to implement only necessary parts. If later we will find a need of this feature, it will be easier to add it than to remove it.
Strictly speaking, making the Placeholder class non-inheritable and non-instantiable is not required. But it is easy to implement.
I hope Raymond will change his opinion on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you are saying makes sense, but at the same time I like current behaviour and if sentinels were standardised and their creation was made more convenient I think this elegant behaviour would be nice to get by default.
I am neutral by now on this specific case. Well, slightly negative just because I put thought and effort into this and simply like it.