deprecate `getproperty(::Pairs, s)` #39448

simeonschaub · 2021-01-29T11:34:08Z

Last triage there was some discussion about overloading getproperty for Pairs{<:NamedTuple} (and perhaps even for AbstractDict{Symbol}). This would break code that accesses internal fields via getproperty, so it would be good to know how much of an impact this would have with a PkgEval run.

simeonschaub · 2021-01-29T13:32:52Z

Hmm, all the test failures in base and stdlib are already not a great sign...

vtjnash · 2021-01-29T17:04:51Z

Seems like one code bug (kw.data === NamedTuple() instead of isempty(kw)) that gets run frequently. That code (70d8497) hasn't even made it into a release yet.

simeonschaub · 2021-01-29T17:41:49Z

It was used in InteractiveUtils and accumulate as well, but you are right, it looked more intimidating than it actually was.

vtjnash

It would be somewhat better to fix these bugs in the code, than just keep them working

base/reduce.jl

stdlib/InteractiveUtils/src/macros.jl

base/accumulate.jl

simeonschaub · 2021-02-01T22:59:33Z

@nanosoldier runtests(ALL, vs=":master")

nanosoldier · 2021-02-02T04:00:07Z

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

simeonschaub · 2021-02-02T15:02:17Z

Looks like this is breaking quite a few packages, so I am not sure that is something we can do for 1.x. Some of the 114 failures seem to be caused by the same packages, but there are still quite a few distinct ones affected.

simeonschaub · 2021-02-04T16:49:59Z

x-ref: #25750

base/iterators.jl

simeonschaub · 2021-04-03T16:07:03Z

Yeah, I agree that making this a deprecation for now is probably the best strategy.

simeonschaub · 2021-04-03T18:00:02Z

base/iterators.jl

+get(v::Pairs, key, default) = get(getfield(v, :data), key, default)
+get(f::Base.Callable, v::Pairs, key) = get(f, getfield(v, :data), key)
+if getproperty !== getfield
+    @deprecate getproperty(x::Pairs, s::Symbol) getfield(x, s)


Suggested change

@deprecate getproperty(x::Pairs, s::Symbol) getfield(x, s)

@eval @deprecate getproperty(x::Pairs, s::Symbol) getfield(x, s)

This should go in deprecated.jl

base/deprecated.jl

vtjnash · 2021-04-16T20:46:26Z

I think triage already approved, so this can be merged in the next couple days

Co-authored-by: Jameson Nash <[email protected]>

quinnj · 2021-05-11T02:30:10Z

I have a couple of use-cases where I define:

nt(; kw...) = kw.data

the deprecation message only mentions keys(kw) or values(kw), so how do we get a NamedTuple from kw? Should I just change them to getfield(kw, :data)?

simeonschaub · 2021-05-11T04:56:00Z

Either values(kw) or NamedTuple(kw) should work. (I think the latter might only work on more recent versions of Julia.)

quinnj#154.

fredrikekre · 2021-05-13T23:17:51Z

base/deprecated.jl

+
+# the plan is to eventually overload getproperty to access entries of the dict
+@noinline function getproperty(x::Pairs, s::Symbol)
+    depwarn("use values(kwargs) and keys(kwargs) instead of kwargs.data and kwargs.itr", :getproperty, force=true)


Why the force=true?

We need a depwarn version system that allows better granularity, but generally force=false means a v2 (breaking) deprecation while force=true means a user bug. This PR adds a warning for a user bug.

KristofferC · 2021-05-14T07:01:25Z

Personally, I think with a forced deprecation warning this is a very annoying change in relation to the benefit it provides.

simeonschaub · 2021-05-19T15:28:23Z

Yes, I think I tend to agree. @vtjnash, are you ok with removing the force=true?

* Fix deprecated .data access on ; kw..., see JuliaLang/julia#39448, fixes #154. * Set version to 1.8.2.

Co-authored-by: Jameson Nash <[email protected]>

Warning: use values(kwargs) and keys(kwargs) instead of kwargs.data and kwargs.itr Comes from JuliaLang/julia#39448

This fixes deprecation of kwargs.data in Julia 1.7 (see JuliaLang/julia#39448)

Fixes the deprecation introduced in JuliaLang/julia#39448 (Julia v1.7+)

simeonschaub added DO NOT MERGE Do not merge this PR! needs pkgeval Tests for all registered packages should be run with this change labels Jan 29, 2021

vtjnash reviewed Jan 29, 2021

View reviewed changes

vtjnash reviewed Apr 3, 2021

View reviewed changes

base/iterators.jl Outdated Show resolved Hide resolved

simeonschaub commented Apr 3, 2021

View reviewed changes

deprecate getproperty(::Pairs, s)

cd01bc1

simeonschaub force-pushed the sds/getprop_pairs branch from d1e659b to cd01bc1 Compare April 13, 2021 21:29

simeonschaub changed the title ~~[DO NOT MERGE] error for getproperty(::Pairs, ...)~~ deprecate getproperty(::Pairs, s) Apr 13, 2021

simeonschaub added deprecation This change introduces or involves a deprecation and removed DO NOT MERGE Do not merge this PR! needs pkgeval Tests for all registered packages should be run with this change labels Apr 13, 2021

vtjnash reviewed Apr 14, 2021

View reviewed changes

base/deprecated.jl Outdated Show resolved Hide resolved

vtjnash marked this pull request as ready for review April 14, 2021 03:04

vtjnash approved these changes Apr 14, 2021

View reviewed changes

simeonschaub added the triage This should be discussed on a triage call label Apr 15, 2021

Update base/deprecated.jl

7fc9b59

Merge remote-tracking branch 'origin/master' into sds/getprop_pairs

5a32dc9

simeonschaub added merge me PR is reviewed. Merge when all tests are passing and removed triage This should be discussed on a triage call labels Apr 27, 2021

simeonschaub merged commit ff001a4 into master Apr 27, 2021

simeonschaub deleted the sds/getprop_pairs branch April 27, 2021 11:27

simeonschaub removed the merge me PR is reviewed. Merge when all tests are passing label Apr 28, 2021

jaakkor2 mentioned this pull request May 3, 2021

Depwarns on Julia 1.7 "use values(kwargs) and keys(kwargs) instead of kwargs.data and kwargs.itr" quinnj/JSON3.jl#154

Closed

ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021

deprecate getproperty(::Pairs, s) (JuliaLang#39448)

f606f24

Co-authored-by: Jameson Nash <[email protected]>

jarlebring pushed a commit to jarlebring/julia that referenced this pull request May 4, 2021

deprecate getproperty(::Pairs, s) (JuliaLang#39448)

e6fc4bb

Co-authored-by: Jameson Nash <[email protected]>

antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021

deprecate getproperty(::Pairs, s) (JuliaLang#39448)

d38be1e

Co-authored-by: Jameson Nash <[email protected]>

xitology mentioned this pull request May 10, 2021

Current Julia master complains about usage of kwargs.data in DBInterface JuliaDatabases/DBInterface.jl#39

Closed

fredrikekre added a commit to fredrikekre/JSON3.jl that referenced this pull request May 13, 2021

Fix deprecated .data access on ; kw..., see JuliaLang/julia#39448, fixes

568c749

quinnj#154.

fredrikekre reviewed May 13, 2021

View reviewed changes

quinnj pushed a commit to quinnj/JSON3.jl that referenced this pull request May 25, 2021

Fix deprecated .data access on ; kw... (#156)

1c69202

* Fix deprecated .data access on ; kw..., see JuliaLang/julia#39448, fixes #154. * Set version to 1.8.2.

blegat mentioned this pull request May 28, 2021

Fix NamedTuple access for Julia nightly jump-dev/PolyJuMP.jl#64

Merged

KristofferC mentioned this pull request May 31, 2021

RFC: remove deprecation of getproperty on Pairs #41027

Closed

JeffFessler mentioned this pull request Jun 26, 2021

deprecated kw.data and nt.names JuliaArrays/AxisArrays.jl#200

Closed

johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021

deprecate getproperty(::Pairs, s) (JuliaLang#39448)

daf9227

Co-authored-by: Jameson Nash <[email protected]>

visr added a commit to visr/AxisKeys.jl that referenced this pull request Jul 26, 2021

fix warning on julia 1.7

4e8e528

Warning: use values(kwargs) and keys(kwargs) instead of kwargs.data and kwargs.itr Comes from JuliaLang/julia#39448

visr mentioned this pull request Jul 26, 2021

fix warning on julia 1.7 mcabbott/AxisKeys.jl#76

Merged

kescobo mentioned this pull request Oct 12, 2021

Kwargs data and unsafe_length Tokazama/AxisIndices.jl#76

Merged

baumgold mentioned this pull request Jan 27, 2022

Fix deprecation of getproperty(::Pairs, s) in Julia 1.7 JuliaData/TableOperations.jl#29

Closed

stuartthomas25 added a commit to stuartthomas25/TypedTables.jl that referenced this pull request Aug 24, 2022

replace kwargs.data to data(kwargs)

7de8b44

This fixes deprecation of kwargs.data in Julia 1.7 (see JuliaLang/julia#39448)

This was referenced Aug 24, 2022

Fix Julia 1.7 deprecation of kwargs.data JuliaData/TypedTables.jl#99

Closed

replace kwargs.data to data(kwargs) stuartthomas25/TypedTables.jl#2

Merged

thevolatilebit mentioned this pull request Sep 7, 2022

fix deprecated call SciML/DiffEqBase.jl#819

Merged

iamed2 mentioned this pull request Dec 1, 2022

Fix kwargs.data deprecation (since Julia 1.7) invenia/Nabla.jl#221

Merged

baumgold mentioned this pull request Mar 22, 2023

Fix deprecation of getproperty(::Pairs, s) apache/arrow-julia#409

Merged

quinnj pushed a commit to apache/arrow-julia that referenced this pull request Mar 23, 2023

Fix deprecation of getproperty(::Pairs, s) (#409)

adf6ab8

Fixes the deprecation introduced in JuliaLang/julia#39448 (Julia v1.7+)

ChrisRackauckas mentioned this pull request Jul 31, 2023

Non-deterministic inference results? #50735

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deprecate `getproperty(::Pairs, s)` #39448

deprecate `getproperty(::Pairs, s)` #39448

simeonschaub commented Jan 29, 2021

simeonschaub commented Jan 29, 2021

vtjnash commented Jan 29, 2021 •

edited

Loading

simeonschaub commented Jan 29, 2021

vtjnash left a comment

simeonschaub commented Feb 1, 2021

nanosoldier commented Feb 2, 2021

simeonschaub commented Feb 2, 2021

simeonschaub commented Feb 4, 2021

simeonschaub commented Apr 3, 2021

simeonschaub Apr 3, 2021

vtjnash Apr 3, 2021

vtjnash commented Apr 16, 2021

quinnj commented May 11, 2021

simeonschaub commented May 11, 2021

fredrikekre May 13, 2021

vtjnash May 14, 2021

KristofferC commented May 14, 2021

simeonschaub commented May 19, 2021

	@deprecate getproperty(x::Pairs, s::Symbol) getfield(x, s)
	@eval @deprecate getproperty(x::Pairs, s::Symbol) getfield(x, s)

deprecate getproperty(::Pairs, s) #39448

deprecate getproperty(::Pairs, s) #39448

Conversation

simeonschaub commented Jan 29, 2021

simeonschaub commented Jan 29, 2021

vtjnash commented Jan 29, 2021 • edited Loading

simeonschaub commented Jan 29, 2021

vtjnash left a comment

Choose a reason for hiding this comment

simeonschaub commented Feb 1, 2021

nanosoldier commented Feb 2, 2021

simeonschaub commented Feb 2, 2021

simeonschaub commented Feb 4, 2021

simeonschaub commented Apr 3, 2021

simeonschaub Apr 3, 2021

Choose a reason for hiding this comment

vtjnash Apr 3, 2021

Choose a reason for hiding this comment

vtjnash commented Apr 16, 2021

quinnj commented May 11, 2021

simeonschaub commented May 11, 2021

fredrikekre May 13, 2021

Choose a reason for hiding this comment

vtjnash May 14, 2021

Choose a reason for hiding this comment

KristofferC commented May 14, 2021

simeonschaub commented May 19, 2021

deprecate `getproperty(::Pairs, s)` #39448

deprecate `getproperty(::Pairs, s)` #39448

vtjnash commented Jan 29, 2021 •

edited

Loading