How do people handle name collisions with function defined in other packages?

As an example I’m using Cairo to create an image. I defined a method:

using Cairo

function image(size::Int)
   ....
end

This function just does some base work for creating an image of size x size for me. However can’t do that do at:

ERROR: LoadError: error in method definition: function Cairo.image must be explicitly imported to be extended

Okay I don’t know what the image() function in Cairo is or does, but I’m not overridden it, improving it, or calling it. I just wanted to create my own function named “image”.

So do people just change the function definition to:

function Cairo.image(size::Int)

Or renamed their function? Maybe prefix all your function with “my_” or something?

I would just change the name of your own function. You can of course extend the existing method, but then it has potential to cause confusion. There is no confusion if you just use a different name, only slight inconvenience.

Yeah, that’s what I’ve been doing, but I feel like I’ve been hitting this conflict more and more recently…I’m not sure why.

To give a more complete answer: you can also just import the functions from Cairo that you do use, if there are only a few of them. Then Cairo.image wouldn’t be in your namespace, and you could define your own image function without issue.

4 Likes

My question here would this be considered “best practice” or is it what people “normally” do?

I think most people probably just use a different name, unless you are actually extending the existing method.

Regarding importing only specific functions, you will definitely see this if you look at source code for packages. Some people might call it best practice because it is the most explicit.

See https://docs.julialang.org/en/v1/manual/modules/#Summary-of-module-usage-1

No you should not have this problem. This should only happen if you have actually used image.

Since you are seeing the error, it means that you are actually using image from Cairo in which case you need to decide why you are using it and if the new function you are defining agrees with that usage. OTOH, if you only have a ::Int parameter then extending Cairo.image is definitely type piracy and you must not do that. You can either change the name of image, since you are actually using two functions, or you can define your own image that fallbacks to Cairo's with image(args...; kws...) = Cairo.image(args...; kws...).

7 Likes

Okay that helps. This was all happening when I was still fixing various coding errors. So at a guess my image method was called when there was an error compiling the my image() method. That pulled in Cairo’s method definition so that when I fixed my image() method, I ended up with a conflict.

To avoid exporting functions/types but still make it easy to use them, I did a couple of tricks in MusicXML that I think are useful: https://github.com/JuliaMusic/MusicXML.jl/blob/6a08b0c98b63cf107adc8094a7377dc6631ff3a8/src/MusicXML.jl#L39

  1. add a importer macro:
 @importMX # gets translated to: import MusicXML: ScorePartwise, Part, Measure, Note, Chord, Unpitched, Rest, Pitch, Attributes, Time, Transpose, Clef, Key, PartList, ScorePart, MidiInstrument, MidiDevice, ScoreInstrument

You can add an option like: @importMX expect_this

  1. Add an alias for your package name:
 export MX
 const MX = MusicXML
  1. A macro for adding MX. to the functions/types:
 export @MX
 macro MX(expr)
     expr_string = string(expr)
     expr_string = reduce(replace,
         (
         r"(?<!\.)\bAttributes\b" => "MX.Attributes",
         r"(?<!\.)\bTime\b" => "MX.Time",
         ),
         init = expr_string)
     return esc(Meta.parse(expr_string))
 end
 @MX Time(stuff)  == Attributes(stuff) # becomes MX.Time(stuff) = MX.Attributes(stuff)

Don’t do this. This kind of code is exactly how you can get into the situation like this. By using using, the user doesn’t have to know anything about any new function being imported. However, if you are importing everything implicitly, a new symbol will always cause a conflict. The symptom of the conflict will not be the same but it’s just as bad (and worse, since you may not get a very obvious error when there’s a conflict)

This is fine. IIRC OpenCL does this too.

The idea of this macro is OK, However, the implementation

is bad. Never, ever use string manipulation in a macro like this. Never. It’s not even that hard to come up with a case that completely break your code.

1 Like

I believe as long as you’ve defined image before using it this would not be a problem. Even if your image is missing method or is raising an error the name resolution shouldn’t fallback to imported ones after your own is defined.

In another word, if you truely don’t need image from Cairo you might have got into this situation in current session by calling image before defining it (i.e. that error has a persistent side-effect). It wouldn’t matter in a new session as long as you defined image before using it. You can still use the image from Cairo by Cairo.image, which, IMO, is a better way to use such a generic name in any case…

1 Like

Yeah, I’m thinking it might just be better to import the packages then call the function prefixed with the package name.

Yes please let this be a reminder to package devs not to use extremely generic names for functions!

I would argue instead for using the generic name but when it is actually package specific just don’t export it and document it as having to be fully qualified to use.

When it’s a simple operation, you should be allowed to use a simple name for it. All what you need is namespace and that’s exactly what modules are… No need to reinvent the wheel by encode that namespace in the name itself.

6 Likes

I don’t see why. It is very nice to compose a small DSL out of short names.

Users of the package should be prepared to manage their namespace. You can’t just keep using everything and expect it to work.

@Tamas_Papp we already know which category you fall into :smirk:

That’s why I made a syntax for it. User importMX only when he is sure that there is no conflict. Otherwise, they just should call regularly.

My code works in any-case that someone has written a valid Julia code. I don’t wanna make my code slower, make developing it hard, and deal with args when I just wanna do a find and replace. There are two solutions for the problem, but I chose the best. This stereotype that “never use string manipulation inside a macro” doesn’t mean much to me when there is no solid rationale behind it.

The point is that how can someone be sure that there is never going to be a conflict in the future? Do you have a list of names that you’ll never use or a list of names that you’ll only use?

(and FWIW, if this is only to be used when there’s no conflict then by all mean it is not a solution to the issue metioned in this thread. If you do have a strict naming rule that you follows, then yes, that’ll make this macro a valid thing to have and that rule itself is a valid, albiet unusual, solution to the issue of name conflict.)

Nope. It doesn’t work if the use have a "Attributes" in the code, or :Attributes FWIW.

It’ll by all mean make your code faster by reducing the amount of work to do since what you want to do is a syntax replacement and not a string one.

It’s not a sterotype. It might not mean much to you right now and if it works for you right now you are certainly free to use it. Just be careful when it bite you inevitably sometime later.

I have little problem when people write code like this and I’ve even done much worse when I know it’s a throw away code. However, it is definitely not some code that you can recommend to other people publically. The correct way to write a micro may not mean much to you but it certainly mean a lot for maintaining a minimum level of code quality over the whole ecosystem.

The names of the types are mentioned in the documentation. This isn’t magic. It is just a simple thing that facilitates importing the user is sure about.

What do you mean? It works as expected.

Any code that includes Attributes is supposed to get MusicXML. behind, unless it is imported explicitly from some package like Foo.Attributes

So are you saying your package will never ever have a new name added?

And just to be clear, I’m talking about compatibility when new functions are added. i.e.

Of course whatever you do, your downstream can always be fixed accordingly. That’s fine for throwaway code or small personal project. However, if you are ever a upstream with hundreds of downstream, doing so unnecessary won’t be acceptable. Having something mentioned in the document has nothing to do with this unfortunately. Currently, documenting the package has exported certain names does not mean that the package does not export more, for exactly the reason that no one I know can predict the future. However, you are making that (absense of future exported names) part of the API that the user can depend on so any addition to the document or the code is now a breaking change. (Hence why I say this is making the issue worse, and in a subtle way)

I assume you meant in front, but that’s not the point. Your code are capturing f() = g("Attribute") and f() = g(:Attribute) where it shouldn’t be

julia> @macroexpand @MX f() = g("Attributes")
:(f() = begin
          #= none:1 =#
          #= none:3 =#
          g("MX.Attributes")
      end)

julia> @macroexpand @MX f() = g(:Attributes)
:(f() = begin
          #= none:1 =#
          #= none:3 =#
          g(:MX.Attributes)
      end)