Skip to content

make @timed return NamedTuple (# 34147) #34149

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jan 8, 2020
Merged

make @timed return NamedTuple (# 34147) #34149

merged 7 commits into from
Jan 8, 2020

Conversation

@jw3126
Copy link
Contributor

@jw3126 jw3126 commented Dec 19, 2019

fix #34147

@KristofferC KristofferC added the needs compat annotation Add !!! compat "Julia x.y" to the docstring label Dec 19, 2019
@ararslan ararslan added the needs tests Unit tests are required for this change label Dec 19, 2019
StefanKarpinski
StefanKarpinski reviewed Dec 19, 2019
View reviewed changes
JeffBezanson
JeffBezanson reviewed Dec 19, 2019
View reviewed changes
base/util.jl Outdated
0.0055765
julia> fieldnames(typeof(memallocs))
julia> fieldnames(typeof(o.gcdiff))
Copy link
Member

@JeffBezanson JeffBezanson Dec 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about gcstats as the name of this field?

Copy link
Member

@StefanKarpinski StefanKarpinski Dec 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the thing about using a named tuple is that now the names are significant, so we need to think about them 😬

@ararslan ararslan removed needs compat annotation Add !!! compat "Julia x.y" to the docstring needs tests Unit tests are required for this change labels Dec 20, 2019
@jw3126
Copy link
Contributor Author

jw3126 commented Dec 20, 2019

I think the CI fail is unrelated.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Dec 20, 2019

Would it be sacrelidge to name these gc_time and gc_stats?

@JeffBezanson
Copy link
Member

JeffBezanson commented Dec 21, 2019

Yes 😛

@jw3126
Copy link
Contributor Author

jw3126 commented Dec 23, 2019

Can somebody rerun CI?

@jw3126
Copy link
Contributor Author

jw3126 commented Dec 27, 2019

Merge?

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Dec 27, 2019

Seems good but I think everyone is taking a break until the new year.

JeffBezanson
JeffBezanson reviewed Jan 3, 2020
View reviewed changes
JeffBezanson
JeffBezanson reviewed Jan 3, 2020
View reviewed changes
jw3126 and others added 2 commits January 3, 2020 21:19
@jw3126 @JeffBezanson
Update base/util.jl
4a1a6b6 
Co-Authored-By: Jeff Bezanson <jeff.bezanson@gmail.com>
@jw3126 @JeffBezanson
Update base/util.jl
e1b840e 
Co-Authored-By: Jeff Bezanson <jeff.bezanson@gmail.com>
@JeffBezanson JeffBezanson added the needs news A NEWS entry is required for this change label Jan 3, 2020
@JeffBezanson JeffBezanson merged commit 41501a5 into JuliaLang:master Jan 8, 2020
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
@jw3126 @KristofferC
make @timed return NamedTuple (# 34147) (#34149)
ec224d4
@ericphanson ericphanson mentioned this pull request Sep 21, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StefanKarpinski StefanKarpinski StefanKarpinski left review comments

@JeffBezanson JeffBezanson JeffBezanson approved these changes

Assignees

No one assigned

Labels

needs news A NEWS entry is required for this change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Return named tuple from @timed

5 participants

@jw3126 @StefanKarpinski @JeffBezanson @KristofferC @ararslan