delete for NamedTuples #27725

JeffreySarnoff · 2018-06-22T06:24:26Z

Given a named tuple and a tuple of field names to omit, provides the redacted named tuple.
see #27574 (comment)

fredrikekre · 2018-06-22T08:08:30Z

Shouldn't this be a method of deleteat?

JeffreySarnoff · 2018-06-22T08:22:55Z

deleteat! exists in Base, deleteat does not. deleteat! seems inappropriate because there is no inplace activity. Do you think it is worth introducing deleteat? Is deletefields or deletenames better than dropnames?

JeffreySarnoff · 2018-06-22T08:23:17Z

the appveyor build ran out of time or memory

andyferris · 2018-06-22T08:37:14Z

+1 for delete (a named tuple is more like a dict than array) or deleteat existing.

JeffreySarnoff · 2018-06-22T10:43:03Z

I like delete, too.
I forked this on github, and have no local copy. If I edit the fork and do nothing else, does this PR update automatically, or is there something more required?

Datseris · 2018-06-22T10:54:34Z

Yes.

I think an export statement would also be nice?

JeffreySarnoff · 2018-06-22T11:29:00Z

name changed to delete
docstring revised accordingly
tests revised to test delete
delete added to exports.jl (under collections)

Datseris · 2018-06-22T11:35:21Z

base/namedtuple.jl

@@ -280,6 +280,16 @@ get(f::Callable, nt::NamedTuple, key::Union{Integer, Symbol}) = haskey(nt, key)
    (names...,)
 end

+"""
+    delete(nt::NamedTuple{an}, omitnames::Tuple{Vararg{Symbol}}) where {an}


@JeffreySarnoff do you think it is really necessary to give the parametric type in the named tuple? I don't think the {an} where {an} adds something and makes the docstring longer and less readable.

I copied the form used in another NamedTuple function. The an captures the names directly, rather than requiring a separate function call.

I am referring to the documentation string. Should the user care about this implementation detail? Isn't it anyway clear that a named tuple is parametricaly defined?

really, though for the docstring could be given without that and still be valid -- just not matching
I have no idea if that is frowned upon.

""" delete(nt::NamedTuple, omitnames::Tuple{Varag{Symbol}}) """

I will shorten the doc string -- and if there are voices to put it back -- I will do that.

JeffBezanson · 2018-06-22T15:02:45Z

I think there should be a method of this that accepts a single symbol, which is more compatible with existing delete! methods.

JeffreySarnoff · 2018-06-22T15:12:55Z

What if it took one or more symbols? That is the way I had first thought of it.

JeffreySarnoff · 2018-06-22T15:23:59Z

There gets to be a time cost with needing to call this twice or more to remove multiple named values.

JeffreySarnoff · 2018-06-22T17:07:28Z

oh, misread -- addressed

JeffreySarnoff · 2018-06-22T23:27:47Z

ci/circleci has been pending for ~6 hours "waiting for jobs [your running builds] to finish", there do not appear to be any running builds for these checks.

JeffreySarnoff · 2018-06-23T02:19:43Z

It says circleci checks failed -- does this have something to do with the changes I suggest?
Here is the end of the log.


Sysimage built. Summary:
Total ─────── 433.265285 seconds 
Base: ───────  98.153646 seconds 22.6544%
Stdlibs: ──── 294.759869 seconds 68.0322%
Precompile: ─  40.347553 seconds 9.31244%
Killed
*** This error is usually fixed by running `make clean`. If the error persists, try `make cleanall`. ***
Makefile:201: recipe for target '/home/circleci/project/usr/lib/julia/sys-o.a' failed
make[1]: *** [/home/circleci/project/usr/lib/julia/sys-o.a] Error 1
Makefile:78: recipe for target 'julia-sysimg-release' failed
make: *** [julia-sysimg-release] Error 2
make: *** Waiting for unfinished jobs....
    LINK usr/lib/julia/sys-debug.so
Exited with code 2

KristofferC · 2018-06-23T09:49:35Z

base/namedtuple.jl

+"""
+    delete(nt::NamedTuple, omitnames::Tuple{Vararg{Symbol}})
+
+Construct a copy of a named tuple `nt`, omitting the fields named in `omitnames`.


Perhaps copy doesn't have to be mentioned since tuples are immutable?

KristofferC · 2018-06-23T09:57:40Z

test/namedtuple.jl

@@ -201,6 +201,15 @@ end
 abstr_nt_22194_3()
 @test Base.return_types(abstr_nt_22194_3, ()) == Any[Any]

+@test Base.delete((a=1, b=2), (:b,)) == (a=1,)


I think I read that the result here should be inferrable? If so, should these test also check that?

How do I test that? Guessed this for that.
@test typeof(delete((a=1.0, b="two"), :b)) == NamedTuple{(:a,), Tuple{Float64}}

@inferred should work.

Also Base. isn't needed, right?

JeffreySarnoff · 2018-06-23T11:51:48Z

@KristofferC thought to test @inferred is on point. The named tuple functions themselves are inferrable, but wrapped inside a function that goes away.

 julia> using Test

julia> nt = (a=1, b="2", c=3.0)
(a = 1, b = "2", c = 3.0)

julia> keepnames = (:a, :c)
(:a, :c)

julia> NamedTuple{keepnames}(nt)
(a = 1, c = 3.0)

julia> @inferred NamedTuple{keepnames}(nt)
(a = 1, c = 3.0)

julia> function wrap(keepnames::NTuple{N,Symbol}, nt::NamedTuple{nms}) where {N, nms}
           NamedTuple{keepnames}(nt)
       end
wrap (generic function with 2 methods)

julia> wrap(keepnames, nt)
(a = 1, c = 3.0)

julia> @inferred wrap(keepnames, nt)
ERROR: return type NamedTuple{(:a, :c),Tuple{Int64,Float64}} does not match inferred return type Any
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] top-level scope at none:0

The same thing happens with structdiff.

JeffreySarnoff · 2019-03-07T08:26:57Z

fyi there is delete for NamedTuples in (NamedTupleTools.jl)[https://github.com/JeffreySarnoff/NamedTupleTools.jl]

pierzy · 2019-03-07T12:01:42Z

Thanks! I will use the suggested solution (NamedTupleTools.jl). Will it ever become part of Base?

JeffreySarnoff · 2019-03-07T12:34:19Z

not my call

arnavs · 2019-04-25T16:53:29Z

FWIW, I just added NamedTupleTools to a project for precisely this function.

@JeffreySarnoff thanks for struggling valiantly with this PR. @StefanKarpinski - it looks like Jeff has complied with triage's decision. From an outside perspective, it looks worth re-opening.

pablosanjose · 2019-11-29T11:51:38Z

One more vote towards reopening and reconsidering this for Base

rafaqz · 2022-03-17T16:43:09Z

It would be good to have this in Base to also have the delete method to extend for any immutable objects.

rafaqz · 2022-03-17T16:47:11Z

Can we reopen this? Now that getindex also accepts a Tuple of Symbol, that shouldn't be controversial to do here.

KronosTheLate · 2022-11-29T08:04:32Z

+1 on reopening this.

JeffreySarnoff · 2022-11-29T08:46:11Z

+𝟐 on this early gift for the New Year

tomchor · 2023-02-02T22:23:54Z

+1 for reopening this

3f6a · 2023-08-23T22:25:45Z

reopen this?

brenhinkeller · 2023-08-23T22:38:56Z

Hmm, not clear if the discussion ever went back to triage after Jeffrey's revisions. I'll add a triage label here and then hopefully it can either be discussed again – or if it already was someone can report back here (or in #27574).

Note though:

JeffreySarnoff · 2023-08-24T13:46:36Z

do not read anything into the fact that my submitting branch was deleted

KronosTheLate · 2023-08-29T07:39:26Z

I am yet to see any reason why this issue is closed, and there is clearly a demand for this functionality. Would whoever makes the calls enlighten users as for why this has not been added?

LilithHafner · 2023-10-27T01:54:22Z

#51098 is already tagged for triage

JeffBezanson · 2023-11-09T03:10:13Z

I don't know why this was closed; I approve this PR.

LilithHafner · 2023-11-09T03:10:48Z

Triage says merge this PR.

Copy the diff to a new PR if GitHub won't let us merge this.

3f6a · 2023-11-28T15:03:16Z

Anyone taking this up in a new PR? Or I can just do it (copying the diff here as suggested by Lilith)

JeffreySarnoff · 2023-11-28T17:49:34Z

There is a PR that I created from the one that was closed for this purpose. I did that a while ago and do not have the link handy. It exists already -- somewhere in PR-land.

…

On Tue, Nov 28, 2023 at 10:03 AM 3f6a ***@***.***> wrote: Anyone taking this up in a new PR? — Reply to this email directly, view it on GitHub <#27725 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAM2VRTLWJT6ZJKP6DZHITTYGX4MDAVCNFSM4FGM2YO2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBTGAYDENZRGU3A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

LilithHafner · 2023-11-28T17:56:09Z

Triage specifically approved the diff in this PR (and not the second PR #51098 (comment))

Yes, @3f6a, you can do it!

CedricTravelletti · 2024-04-29T17:22:04Z

+1 for reopening

LilithHafner · 2024-04-30T12:22:28Z

This PR cannot be reopened right not for technical reasons, but if you would like to see this happen you may open a new PR with the same content.

from #27725 Co-authored-by: Jeffrey Sarnoff <[email protected]>

from JuliaLang#27725 Co-authored-by: Jeffrey Sarnoff <[email protected]>

JeffreySarnoff added 2 commits June 22, 2018 02:14

add dropnames

e747501

add dropnames tests

fdd3a98

JeffreySarnoff mentioned this pull request Jun 22, 2018

Method for dropping an element of NamedTuple #27574

Closed

JeffreySarnoff added 3 commits June 22, 2018 07:24

change dropnames to delete

a239411

change dropnames to delete

36af455

export delete (namedtuples)

e33cea2

Datseris reviewed Jun 22, 2018

View reviewed changes

shorten docstring for delete

fc46f7a

JeffreySarnoff added 3 commits June 22, 2018 13:04

add delete(::NamedTuple, ::Symbol)

0338959

add tests for delete(::NamedTuple, ::Symbol)

0bc6e80

another test for delete(::NamedTuple, ::Symbol)

354d543

fredrikekre changed the title ~~dropnames for NamedTuples~~ delete for NamedTuples Jun 22, 2018

KristofferC reviewed Jun 23, 2018

View reviewed changes

JeffreySarnoff added 3 commits June 23, 2018 06:07

reword delete docstring for clarity

cf160bb

rework delete docstring for clarity

ec186c5

test typeof(delete(..)) is fully specified

524bc8b

brenhinkeller added the status:triage This should be discussed on a triage call label Aug 23, 2023

JeffreySarnoff mentioned this pull request Aug 29, 2023

add delete for key[s] in a NamedTuple #51098

Closed

LilithHafner removed the status:triage This should be discussed on a triage call label Oct 27, 2023

JeffBezanson added a commit that referenced this pull request Jul 26, 2024

add delete for NamedTuple

2c6c9e4

from #27725 Co-authored-by: Jeffrey Sarnoff <[email protected]>

JeffBezanson mentioned this pull request Jul 26, 2024

add delete for NamedTuple #55270

Merged

JeffBezanson added a commit that referenced this pull request Jul 26, 2024

add delete for NamedTuple

9d13540

from #27725 Co-authored-by: Jeffrey Sarnoff <[email protected]>

JeffBezanson added a commit that referenced this pull request Jul 31, 2024

add delete for NamedTuple (#55270)

dcd8cad

from #27725 Co-authored-by: Jeffrey Sarnoff <[email protected]>

lazarusA pushed a commit to lazarusA/julia that referenced this pull request Aug 17, 2024

add delete for NamedTuple (JuliaLang#55270)

fd3ca49

from JuliaLang#27725 Co-authored-by: Jeffrey Sarnoff <[email protected]>

This pull request was closed.

delete for NamedTuples #27725

delete for NamedTuples #27725

Conversation

JeffreySarnoff commented Jun 22, 2018 • edited Loading

fredrikekre commented Jun 22, 2018

JeffreySarnoff commented Jun 22, 2018 • edited Loading

JeffreySarnoff commented Jun 22, 2018

andyferris commented Jun 22, 2018

JeffreySarnoff commented Jun 22, 2018 • edited Loading

Datseris commented Jun 22, 2018

JeffreySarnoff commented Jun 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented Jun 22, 2018

JeffreySarnoff commented Jun 22, 2018

JeffreySarnoff commented Jun 22, 2018

JeffreySarnoff commented Jun 22, 2018

JeffreySarnoff commented Jun 22, 2018

JeffreySarnoff commented Jun 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffreySarnoff Jun 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffreySarnoff commented Jun 23, 2018

JeffreySarnoff commented Mar 7, 2019

pierzy commented Mar 7, 2019

JeffreySarnoff commented Mar 7, 2019

arnavs commented Apr 25, 2019

pablosanjose commented Nov 29, 2019

rafaqz commented Mar 17, 2022

rafaqz commented Mar 17, 2022

KronosTheLate commented Nov 29, 2022

JeffreySarnoff commented Nov 29, 2022

tomchor commented Feb 2, 2023

3f6a commented Aug 23, 2023

brenhinkeller commented Aug 23, 2023

JeffreySarnoff commented Aug 24, 2023

KronosTheLate commented Aug 29, 2023

LilithHafner commented Oct 27, 2023

JeffBezanson commented Nov 9, 2023

LilithHafner commented Nov 9, 2023

3f6a commented Nov 28, 2023 • edited Loading

JeffreySarnoff commented Nov 28, 2023 via email

LilithHafner commented Nov 28, 2023

CedricTravelletti commented Apr 29, 2024

LilithHafner commented Apr 30, 2024

JeffreySarnoff commented Jun 22, 2018 •

edited

Loading

JeffreySarnoff commented Jun 22, 2018 •

edited

Loading

JeffreySarnoff commented Jun 22, 2018 •

edited

Loading

JeffreySarnoff commented Jun 22, 2018 •

edited

Loading

JeffreySarnoff Jun 23, 2018 •

edited

Loading

3f6a commented Nov 28, 2023 •

edited

Loading