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

Play nicely with other struct def macros #4

Open
jonniedie opened this issue Sep 26, 2020 · 12 comments
Open

Play nicely with other struct def macros #4

jonniedie opened this issue Sep 26, 2020 · 12 comments

Comments

@jonniedie
Copy link
Owner

@concrete should work with Base.@kwdef and similar struct definition macros.

@jonniedie
Copy link
Owner Author

So Base.@kwdef already works with with @concrete

Base.@kwdef @concrete struct Blah
    a
    b
end
julia> blah = Blah(a=1, b="hey")
Blah{Int64,String}(1, "hey")

It doesn't work with the terse keyword, though. Maybe it is going to be better to make a @concrete_terse macro so this isn't a problem. That might just be a good idea anyway. The keyword thing is kinda weird.

@jonniedie
Copy link
Owner Author

I’ll bump myself here because I wanted this again today.

@jonniedie
Copy link
Owner Author

Hopefully dd869e0 does it. Sill doesn't work with the terse keyword, though

@jonniedie
Copy link
Owner Author

Parameters.@with_kw still doesn't work either.

@genkuroki
Copy link

Thank you for making the @concrete macro working with Base.@kwdef. It's very convenient and helpful.

I have independently tried to make ConcreteStructs.jl and Parameters.jl work together well.

My personal conclusion: It is possible to do so by making the following two changes.

  • Change @concrete not creating the inner constructor, so that the Foo{__T_a, __T_b, __T_c}(a, b, c)-type default constructor will be defined.
  • Change Parameters.@with_kw expanding macros in the argument, like Base.@kwdef.

Then @concrete works well with Parameters.@with_kw and more completely with Base.@kwdef.

For details, see my Jupyter notebook.

I am a beginner in programming, so I may be doing something wrong, but I hope it will be helpful.

@jonniedie
Copy link
Owner Author

Thanks for working on this! I'm trying to remember why that inner constructor was there in the first place. I am all for removing it if it's just getting in the way, though. Let me see if I can remember why I have it set up that way.

@janneshb
Copy link

janneshb commented Nov 10, 2022

I implemented something for @concrete to basically commute with any other well-implemented struct def macro by internally shuffling around the order in which the macros are applied.

I'm happy to open a PR so you can have a look. Right now, I don't have the right permissions, though.

@jonniedie
Copy link
Owner Author

@janneshb ha! I didn’t notice it was you at first. Yeah, a PR would be great if you have it handy. I think you should be able to open one without permissions? I just have to approve the CI to run.

@janneshb
Copy link

Hm, it shows I can open a PR, but I don't have permission to push to a remote branch on the repo. Which I would need to do to open a PR, right?

@jonniedie
Copy link
Owner Author

Oh yeah, it'll make you fork your own copy and will be used for the PR.

@ParadaCarleton
Copy link

I implemented something for @concrete to basically commute with any other well-implemented struct def macro by internally shuffling around the order in which the macros are applied.

I'm happy to open a PR so you can have a look. Right now, I don't have the right permissions, though.

Did you ever open that PR? Looks like Parameters' keyword arguments macro doesn't work with @concrete.

@janneshb
Copy link

I did now.
You can check it out here.

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

No branches or pull requests

4 participants