Skip to content

Conversation

@christopher-dG
Copy link

@christopher-dG christopher-dG commented May 14, 2021

This is kind of a stopgap for my "do stuff with extra keys" use case, since it's just a lot easier to implement.

Interestingly, the documentation for applyfield already says "returns false when nm isn't a valid field on x", so maybe we should just do hasfield(T, nm) || return false right at the beginning?

@DilumAluthge
Copy link
Member

Bump @quinnj

I think @christopher-dG and @fchorney are interested in something like this for use in GitForge.jl.

@quinnj
Copy link
Member

quinnj commented Aug 29, 2021

Hmmm, why didn't CI run on this PR?

quinnj added a commit that referenced this pull request Aug 29, 2021
`applyfield` was inconsistent with `applyfield!` in that it threw an
error on non-existent fields rather than return `false`, which the
docuementation _said_ it did. This fixes `applyfield` to ignore
non-existent fieldnames and return `false`. Alternative fix to adding
teh `ignoreextras` function proposed in #52.
@quinnj
Copy link
Member

quinnj commented Aug 29, 2021

Alternative proposal to just fix applyfield here: #63. applyfield is definitely wrong here in not returning false on non-existent fields.

@quinnj quinnj closed this Aug 29, 2021
quinnj added a commit that referenced this pull request Aug 30, 2021
* Fix applyfield when applied to non-existent fields

`applyfield` was inconsistent with `applyfield!` in that it threw an
error on non-existent fields rather than return `false`, which the
docuementation _said_ it did. This fixes `applyfield` to ignore
non-existent fieldnames and return `false`. Alternative fix to adding
teh `ignoreextras` function proposed in #52.

* fix test
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

Successfully merging this pull request may close these issues.

3 participants