deprecated symbol(x...) becomes Symbol(x...) #15995

jeffreysarnoff-dev · 2016-04-22T02:05:12Z

This renaming follows Julia's capitalization of named Types and their constructors.
This pull request should reflect edits to deprecate.jl and expr.jl.

This renaming follows Julia's capitalization of named Types and their constructors.

yuyichao · 2016-04-22T02:15:57Z

I believe you need to update the def of Symbol.

JeffBezanson · 2016-04-22T02:20:42Z

👍

Yes, there are many uses of symbol that will need to be updated, and the definitions in base/expr.jl should be switched to add methods to Symbol.

nalimilan · 2016-04-22T08:51:09Z

@jeffreysarnoff-dev If you wonder why tests failed, deprecation warnings are turned into failures in Travis/AppVeyor.

jeffreysarnoff-dev · 2016-04-22T15:54:16Z

I had changed the the methods in base/expr.jl -- Somehow that file did not get included with the pull request. Please close this pull request. I will process all the source files locally to catch each use of 'symbol' and put the changes into a new fork then resubmit the request. Thank you.

nalimilan · 2016-04-22T15:57:40Z

We can leave this PR open, and just force-push when you're ready.

pull request #15995

This reverts commit 5c2085a.

use Symbol(x...) for deprecated symbol(x...)

jeffreysarnoff-dev · 2016-04-23T06:03:17Z

I have pushed changes to ~67 files (.jl, .md, .rst) relative to ./julia (above /julia/base):
(recursively) replace "symbol(" with "Symbol(" then replace "_Symbol(" with "_symbol".
I see this branch as https://github.com/jeffreysarnoff-dev/julia/tree/patch-3

nalimilan · 2016-04-23T08:41:48Z

Looks good. Unfortunately, changes to the same code areas have been merged into master since you forked. Could you rebase on master and fix conflicts? You should also squash into a single commit.

jeffreysarnoff-dev · 2016-04-23T16:23:53Z

@nalimilan Now that I know how to do it, I can redo it as a single commit to a current fork. It is much easier to do that than to fiddle with conflicts (I have not done that before) and combining multiple commits into one (which did not work for me last time).
Unless you tell me otherwise .. I am going to delete my patch-3 branch on my fork, delete my fork, refork and create a branch called patch-3 then single commit symbol(x...) --> Symbol(x...).
[If I am not permitted to delete and recreate patch-3 on my fork I will try rebasing it from the current state of Julia and then do the rest. I would prefer not, as I have never fully trusted git's ability to understand what I want it to do through what I can ask it to do .]

timholy · 2016-04-23T16:53:57Z

Whatever's easiest is fine. However, if you want to try to use git, do something like this:

git checkout -b patch-3a
git fetch                                  # update origin/master
git rebase -i origin/master
# Now edit the TODO list, using "pick" for the first and "squash" for the rest
# Save and close the file
# Fix any merge conficts
git rebase --continue
git push myfork +patch-3a:/patch-3  # force-push local branch patch-3a to remote branch patch-3

where myfork is the name of your remote (e.g., jeffreysarnof-dev).

stevengj · 2016-04-23T23:00:54Z

base/dft.jl

@@ -264,7 +264,7 @@ A_mul_B!(y::AbstractArray, p::ScaledPlan, x::AbstractArray) =
 # or odd).

 for f in (:brfft, :irfft)
- pf = symbol(string("plan_", f))
+ pf = Symbol(string("plan_", f))


Note that usages like this should probably be replaced with Symbol("plan_", f).

stevengj · 2016-04-23T23:08:07Z

(Good to take this opportunity to clean up a lot of the Symbol calls to remove extraneous calls to string etcetera.)

nalimilan · 2016-04-24T10:16:05Z

Note that all improvements noted by @stevengj would better stay in a separate commit, so that one commit only does the systematic renaming.

jeffreysarnoff-dev · 2016-04-25T14:08:37Z

symbol( --> Symbol then _Symbol( --> _symbol then deprecated.jl done in one commit

jeffreysarnoff-dev · 2016-04-25T14:09:31Z

https://github.com/jeffreysarnoff-dev/julia/tree/patch-3

jeffreysarnoff-dev · 2016-04-25T14:16:48Z

I just noticed that I forgot to include the pull request number in deprecated.jl.
I don`t know how to change that in the same commit -- if you do, please do.
Otherwise, tell me to do it over again.

nalimilan · 2016-04-25T14:18:49Z

@JeffreySarnoff Just make the changes, then git commit -a --amend to add this change to the last commit. But you should either push your changes here or open a new PR, as currently the changes are only on the branch.

jeffreysarnoff-dev · 2016-04-25T14:29:41Z

fixed. should be good to go.

JeffreySarnoff · 2016-04-25T14:30:42Z

thank you -- done

On Mon, Apr 25, 2016 at 10:19 AM, Milan Bouchet-Valat <
[email protected]> wrote:

@JeffreySarnoff https://github.com/JeffreySarnoff Just make the
changes, then git commit -a --amend to add this change to the last
commit. But you should either push your changes here or open a new PR, as
currently the changes are only on the branch.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#15995 (comment)

nalimilan · 2016-04-25T14:41:09Z

Sorry, there are still conflicts with master. See how many commits are included in this branch. If that's easier, open a new clean PR.

Also, do you plan to address @stevengj's enhancements while you're at it?

jeffreysarnoff-dev · 2016-04-25T14:47:05Z

I will use a new PR and a new branch -- I can address @stevenj's enhancements, are you saying 1 PR two commits or 2 PRs?

nalimilan · 2016-04-25T14:48:56Z

1 PR, 2 commits is the best solution.

nalimilan · 2016-04-25T17:43:44Z

Closing in favor of #16038.

nalimilan · 2016-05-01T20:32:46Z

base/strings/basic.jl

@@ -29,7 +29,7 @@ convert(::Type{Vector{UInt8}}, s::AbstractString) = bytestring(s).data
 convert(::Type{Array{UInt8}}, s::AbstractString) = bytestring(s).data
 convert(::Type{ByteString}, s::AbstractString) = bytestring(s)
 convert(::Type{Vector{Char}}, s::AbstractString) = collect(s)
-convert(::Type{Symbol}, s::AbstractString) = symbol(s)
+convert(::Type{Symbol}, s::AbstractString) = Symbol(s)


I guess we should remove this one. Calling the constructor from convert doesn't make sense.

removing this line gave lots of errors! Presumably we're using this in quite a few places.

@nalimilan Can you explain? The docs on constructors seems to suggest that Symbol(a) and convert(Symbol, a) should be equivalent. By default the constructor is defined in terms of convert, but in this case the implementation (that I could not find) seems to be in the constructor.

Sorry, I meant that we usually define convert and let the default constructor fall back to it, rather than the reverse. But in this case that seems to make sense.

From comments by stevengj in JuliaLang#15995.

nalimilan · 2016-05-02T10:27:22Z

base/expr.jl

-symbol(s::ASCIIString) = symbol(s.data)
-symbol(s::UTF8String) = symbol(s.data)
-symbol(a::Array{UInt8,1}) =
+Symbol(s::Symbol) = s


OTOH this constructor is probably useless due to the fallback to convert.

Is this what's causing the method error(s) @tkelman noticed?

...but like I say removing the convert kills the build. :/

As @nalimilan said, the default constructor calls convert, not the other way around, so if you remove something it should be the constructor(s).

jeffreysarnoff-dev added 2 commits April 21, 2016 22:00

replace deprecated symbol(x...) with Symbol(x...)

d346dbe

This renaming follows Julia's capitalization of named Types and their constructors.

deprecated symbol(x...) becomes Symbol(x...)

a0c71c9

This renaming follows Julia's capitalization of named Types and their constructors.

jeffreysarnoff-dev and others added 8 commits April 22, 2016 13:07

Symbol(x...) replaces deprecated symbol(x...)

e268b6a

pull request #15995

pull request #15995 (symbol ==> Symbol)

330fb30

pull request #15994 (symbol ==> Symbol)

e9e2cc9

pull request #15994 deprecate symbol(x...) ==> Symbol(x..)

8e285da

Merged branch patch-3 into patch-2

5c2085a

Revert "Merged branch patch-3 into patch-2"

9b0a2d1

This reverts commit 5c2085a.

use Symbol(x...) for deprecated symbol(x...)

89b233f

pull request #15995

007fe4f

use Symbol(x...) for deprecated symbol(x...)

stevengj reviewed Apr 23, 2016
View reviewed changes

jeffreysarnoff-dev mentioned this pull request Apr 25, 2016

#16038 deprecate symbol ==> Symbol #16038

Closed

nalimilan closed this Apr 25, 2016

hayd mentioned this pull request Apr 30, 2016

PR 16130 deprecate symbol ==> Symbol #16130

Closed

hayd added a commit to hayd/julia that referenced this pull request May 1, 2016

Fix up JuliaLang#15995 with @Stevegj's comments.

360883f

hayd mentioned this pull request May 1, 2016

Deprecate symbol (to Symbol) #16154

Merged

nalimilan reviewed May 1, 2016
View reviewed changes

hayd added a commit to hayd/julia that referenced this pull request May 1, 2016

Clean up Symbol usage (after symbol deprecation).

01c36d7

From comments by stevengj in JuliaLang#15995.

hayd added a commit to hayd/julia that referenced this pull request May 1, 2016

Clean up Symbol usage (after symbol deprecation).

b793476

From comments by stevengj in JuliaLang#15995.

hayd added a commit to hayd/julia that referenced this pull request May 1, 2016

Clean up Symbol usage (after symbol deprecation).

df832b7

From comments by stevengj in JuliaLang#15995.

hayd added a commit to hayd/julia that referenced this pull request May 1, 2016

Clean up Symbol usage (after symbol deprecation).

a9df7e9

From comments by stevengj in JuliaLang#15995.

nalimilan reviewed May 2, 2016
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deprecated symbol(x...) becomes Symbol(x...) #15995

deprecated symbol(x...) becomes Symbol(x...) #15995

jeffreysarnoff-dev commented Apr 22, 2016

yuyichao commented Apr 22, 2016

JeffBezanson commented Apr 22, 2016

nalimilan commented Apr 22, 2016

jeffreysarnoff-dev commented Apr 22, 2016

nalimilan commented Apr 22, 2016

jeffreysarnoff-dev commented Apr 23, 2016 •

edited

Loading

nalimilan commented Apr 23, 2016

jeffreysarnoff-dev commented Apr 23, 2016 •

edited

Loading

timholy commented Apr 23, 2016

stevengj Apr 23, 2016

stevengj commented Apr 23, 2016

nalimilan commented Apr 24, 2016

jeffreysarnoff-dev commented Apr 25, 2016

jeffreysarnoff-dev commented Apr 25, 2016

jeffreysarnoff-dev commented Apr 25, 2016

nalimilan commented Apr 25, 2016

jeffreysarnoff-dev commented Apr 25, 2016

JeffreySarnoff commented Apr 25, 2016

nalimilan commented Apr 25, 2016

jeffreysarnoff-dev commented Apr 25, 2016

nalimilan commented Apr 25, 2016

nalimilan commented Apr 25, 2016

nalimilan May 1, 2016

hayd May 2, 2016

ivarne May 2, 2016

nalimilan May 2, 2016

nalimilan May 2, 2016

hayd May 2, 2016

stevengj May 2, 2016

deprecated symbol(x...) becomes Symbol(x...) #15995

deprecated symbol(x...) becomes Symbol(x...) #15995

Conversation

jeffreysarnoff-dev commented Apr 22, 2016

yuyichao commented Apr 22, 2016

JeffBezanson commented Apr 22, 2016

nalimilan commented Apr 22, 2016

jeffreysarnoff-dev commented Apr 22, 2016

nalimilan commented Apr 22, 2016

jeffreysarnoff-dev commented Apr 23, 2016 • edited Loading

nalimilan commented Apr 23, 2016

jeffreysarnoff-dev commented Apr 23, 2016 • edited Loading

timholy commented Apr 23, 2016

Choose a reason for hiding this comment

stevengj commented Apr 23, 2016

nalimilan commented Apr 24, 2016

jeffreysarnoff-dev commented Apr 25, 2016

jeffreysarnoff-dev commented Apr 25, 2016

jeffreysarnoff-dev commented Apr 25, 2016

nalimilan commented Apr 25, 2016

jeffreysarnoff-dev commented Apr 25, 2016

JeffreySarnoff commented Apr 25, 2016

nalimilan commented Apr 25, 2016

jeffreysarnoff-dev commented Apr 25, 2016

nalimilan commented Apr 25, 2016

nalimilan commented Apr 25, 2016

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffreysarnoff-dev commented Apr 23, 2016 •

edited

Loading

jeffreysarnoff-dev commented Apr 23, 2016 •

edited

Loading