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

Start threading Structured signature into existing datastructures. #291

Merged
merged 21 commits into from
Oct 2, 2023

Conversation

Carreau
Copy link
Member

@Carreau Carreau commented Sep 22, 2023

There are many objects that are called "Signature", Just a draft to try to rename the current one to "TextSignature", to differentiate, and try to pass the current one to be serialized/loaded.

The format is compatible with the format produced by Griffe, except we don't
use an ast form for the annotations.
@asmeurer
Copy link
Collaborator

I was thinking to just store the text form on the Signature object, or to have a method to reconstruct it if enough information is already there in the structured form.

As I suggested in the issue, I think it's a good idea to still include the str form in the JSON alongside the structured form.

@asmeurer
Copy link
Collaborator

Might be a good idea to merge this and #289 together to avoid conflicts.

@Carreau
Copy link
Member Author

Carreau commented Sep 28, 2023

I was thinking to just store the text form on the Signature object, or to have a method to reconstruct it if enough information is already there in the structured form.

As I suggested in the issue, I think it's a good idea to still include the str form in the JSON alongside the structured form.

Yes, I was just experimenting on seeing how far I could get for simpler signature, and ended up going further than I thoughts

Might be a good idea to merge this and #289 together to avoid conflicts.

It's on my todo. I was not planning to go that far and tried to stay away from this file, but ended up touch it anyway. I'll try to find some time to merge the two, or split this one in smaller chunks.

@Carreau Carreau marked this pull request as ready for review October 2, 2023 13:51
@Carreau Carreau merged commit a077b0b into jupyter:main Oct 2, 2023
10 checks passed
@Carreau Carreau deleted the thread-sig branch December 25, 2023 11:29
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.

2 participants