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

[Feature Request] Use @doc_private on initializers from MLIR types to hide them from the API docs #3563

Open
1 task done
soraros opened this issue Sep 29, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed mojo-repo Tag all issues with this label

Comments

@soraros
Copy link
Contributor

soraros commented Sep 29, 2024

Review Mojo's priorities

What is your request?

As title. Suggested by @lattner on Discord.

More concretely, we should change

fn __init__(inout self, value: Self._mlir_type): ...

to

fn __init__(inout self, *, _mlir: Self._mlir_type): ...

What is your motivation for this change?

So we can hide them in docgen or maybe intellisense to avoid, well, this:
Screenshot 2024-09-29 at 06 16 12

Any other details?

We lose implicit conversion from MLIR values this way, I don't know how important that is.

@soraros soraros added enhancement New feature or request mojo-repo Tag all issues with this label labels Sep 29, 2024
@lattner
Copy link
Collaborator

lattner commented Sep 29, 2024

+1 thank you for filing this! This would be a great improvement to make MLIR stuff an implementation detail. We should also look for the var value : Self._mlir_type pattern and upgrade it to _value for similar reasons.

@soraros
Copy link
Contributor Author

soraros commented Sep 30, 2024

@lattner Actually, we already have a decorator @doc_private to do this, let's just put them on all the MLIR constructors.

I don't think making the arguments keyword only is that good though. Having implicit conversion in this case can be quite useful.

Copy link
Collaborator

lattner commented Sep 30, 2024

Oh nice, I didn't realize we had @doc_private. I agree, that seems like a good solution!

Copy link
Collaborator

Yeah, we should more liberally apply @doc_private for things like this.

@lattner lattner changed the title [Feature Request] Move initialisers from MLIR types to use keyword only arguments [Feature Request] Use @doc_private on initializers from MLIR types to hide them from the API docs Oct 2, 2024
@JoeLoser
Copy link
Collaborator

JoeLoser commented Oct 2, 2024

A simple grep like `git grep 'mlir' stdlib | grep -i 'fn init' should find all the ones to go chase down if anyone is interested.

@JoeLoser JoeLoser added help wanted Extra attention is needed good first issue Good for newcomers labels Oct 2, 2024 — with Linear
@miguelcsx
Copy link

Hi @JoeLoser , I started working on this issue, could you assign it to me? thanks 🚀

@JoeLoser
Copy link
Collaborator

JoeLoser commented Oct 7, 2024

Hi @JoeLoser , I started working on this issue, could you assign it to me? thanks 🚀

Done, thanks for starting on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed mojo-repo Tag all issues with this label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants