Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reconstruct with all fieldnames #7

Merged
merged 2 commits into from
Jan 20, 2021

Conversation

ToucheSir
Copy link
Member

This ensures that important, non-fmappable properties with non-default values are preserved. Should address #6 and #3 (comment).

I believe this is theoretically breaking, but a search on JuliaHub doesn't turn up any uses of the macro that specify a list of fields.

This ensures that important, non-fmappable properties with non-default values are preserved.
@devmotion
Copy link

I guess the sentence "For a discussion regarding implementing functors for which only a subset of the fields are "seen" by functor, see here." could be removed from the README and replaced with an example.

@DhairyaLGandhi
Copy link
Member

Seems sensible at a first glance. Thanks!

@ToucheSir
Copy link
Member Author

Updated docs per @devmotion's comment. @DhairyaLGandhi can we cut a release with this so that Flux can start using it? I've seen a fair few PRs lately that stand to benefit.

@CarloLucibello
Copy link
Member

@DhairyaLGandhi bump

@DhairyaLGandhi
Copy link
Member

Seems alright. This would make a breaking release though.

@DhairyaLGandhi DhairyaLGandhi merged commit 43ae832 into FluxML:master Jan 20, 2021
@CarloLucibello
Copy link
Member

tag?

@ToucheSir
Copy link
Member Author

Yup, I was thinking a 0.2 so as to make it in time for Flux 0.12 (or barring that, 0.13)

@DhairyaLGandhi
Copy link
Member

Cool, good that we're in agreement then

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants