Skip to content

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

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

Closed
wants to merge 6 commits into from

Conversation

JeffreySarnoff
Copy link
Contributor

@JeffreySarnoff JeffreySarnoff commented Aug 29, 2023

Replaces #27725

This PR updates delete for NamedTuples. See that (closed) PR for much discussion. This revision supports deleting one or more keys from a NamedTuple.

@aplavin
Copy link
Contributor

aplavin commented Aug 29, 2023

See also #46453 for a more general PR - includes delete(namedtuple) and more. Looks like implementations are somewhat different, which one is "better"?

@DilumAluthge DilumAluthge added the triage This should be discussed on a triage call label Aug 29, 2023
(a = 1,)
```
"""
function delete(a::NamedTuple{an}, @nospecialize(key::Symbol)) where {an}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there no specialize on key::Symbol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I am removing it.

NamedTuple{names}(a)
end

function delete(a::NamedTuple{an}, @nospecialize(keys::Tuple{Vararg{Symbol}})) where {an}
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's a pain, but there should probably be a serious conversation concerning when tuples of indices are used and how they're interpreted. We have tuples of indices for a single index on multidim arrays, but how should we refer to multiple indices on tuples and named tuples? There's a loss of information if we require it always be another subtype of AbstractArray

@@ -1,7 +1,7 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

export
# Modules
# Modules
Copy link
Member

Choose a reason for hiding this comment

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

Please do not change formatting unecessarily

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that was unintentional -- thank you for catching the mistype

@JeffreySarnoff
Copy link
Contributor Author

the test failures do not involve namedtuples.jl

@JeffreySarnoff
Copy link
Contributor Author

This PR introduces delete to convey that the result is newly constructed (and the original is unaltered).
Perhaps omit a the better name (considering @Tokazama)?

experimental keep and omit for NamedTuples. both accept (a) one key or index and (b) a Tuple of keys or indices.

@brenhinkeller brenhinkeller added the collections Data structures holding multiple items, e.g. sets label Sep 15, 2023
@JeffBezanson
Copy link
Member

This has many accidental formatting changes. No matter; the old version of the PR is just fine and we can merge that (it needs to be re-created though).

@LilithHafner LilithHafner removed the triage This should be discussed on a triage call label Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collections Data structures holding multiple items, e.g. sets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants