-
Notifications
You must be signed in to change notification settings - Fork 763
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
New #[pyclass] system that is aware of its concrete layout #683
Merged
Merged
Changes from 13 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
4b5fa7e
Introduce PyClass trait and PyClassShell
kngwyu bdb66af
Make PyClassShell have dict&weakref
kngwyu 4d7dfaf
Allow slf: &PyClassShell<Self>
kngwyu a663907
Introduce PyInternalCaster
kngwyu b86de93
Introduce PyClassInitializer
kngwyu 8175d6f
Merge branch 'master' into pyclass-new-layout
kngwyu 6b84401
Make it enable to safely inherit native types
kngwyu efa16a6
Fix documents accompanied by PyClassShell
kngwyu e2dc843
Fix a corner case for PyClassInitializer
kngwyu acb1120
Fix examples with the new #[new] API
kngwyu d5cff05
Fix documents and a clippy warning
kngwyu ea51756
Resolve some clippy complains
kngwyu 2e3ece8
Try to enhance class section in the guide
kngwyu 5859039
Fix accidently changed file permission
kngwyu 766a520
Documentation enhancement
kngwyu 8f8785d
Merge branch 'master' into pyclass-new-layout
kngwyu 18e565f
New PyClassInitializer
kngwyu 60edeb8
Simplify IntoInitializer
davidhewitt b04d0af
Merge pull request #1 from davidhewitt/pyclass-new-layout
kngwyu b602b4b
Enhance documentation and tests around #[new]
kngwyu f26e07c
Replace IntoInitializer<T> with Into<PyClassInitializer<T>>
kngwyu 67a98d6
Remove unnecessary Box
kngwyu ab0a731
Fix use order in prelude
kngwyu 451de18
Merge branch 'master' into pyclass-new-layout
kngwyu c57177a
Refine tests and documents around pyclass.rs
kngwyu 302b3bb
Merge branch 'master' into pyclass-new-layout
kngwyu 439efbb
Update CHANGELOG
kngwyu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
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'm cautious about including the generated implementation in the doc; it seems like something which could easily fall out of sync with actual code if we made any internal changes in the future.
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.
So what do you think is a good way to tell users what it does?
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 personally like this, but it would be nice if it were possible to add a test that checks that the two don't fall out of sync. I don't know of a great way to do this, but a quick-and-dirty hack might be, be something like:
Then pull the text between the comments and pull the code out from the rust block inside the comments and compare it (possibly in a more abstract way) to the code generated from an equivalent
pyclass
. Maybe that's too much work for what it buys you, though...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.
Also, note, I would end this sentence with
:
instead of.
if you keep it.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 do also like this. It is helpful for users to see it. We will just need to be careful to keep it up to date in future.
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.
Hmm... 🤔
I think it's a rough skecth and just 'compile pass' is suffcient.