Even though the first version is shorter, I still prefer the clarity of the second version, but unfortunately it fails for symbols like '. Is there a way to get the second version to work or is there a third version that is even better?
In my case, I feel that this would be punning. The docstring for isopen seems to be intended for streams and things like that. In my case, I want to use it for querying if a market was open at a specific time. I feel that these two uses are so different that I do not wish to just make another method for Base.isopen, but rather make a MyPackage.isopen function. By not importing it, I am doing what I can to avoid confusion without having to use another name. I do also require using my own type as one of the arguments, so erroneous use should throw an exception.
Of the two approaches in the original post, the first seems much more idiomatic to me and also much safer. What was the issue with the first approach?
A huge hazard of importing all the methods in Base is that any function you define that happens to coincide with an imported Base function (in this case every public function, of which there are currently more than 1000) will add a method to the Base version instead of defining a new function (because an explicitly imported function does not require namespace qualification to extend). And note that more public functions will be added to Base in the future. Which means at any point in the future (if not present), you might accidentally pirate Base functions and break something in a hilarious or not-so-hilarious way.
It is difficult to describe why I don’t like the first solution so much, one thing is that it does things in the wrong order, importing, exporting and using should be done before defining functions, another thing is that I feel that I am abusing “defining function code” as a way to protect against importing a symbol. But I agree that the method in my second post introduces real vulnerabilities, so it is probably an even worse idea.
@liuyxpp I did also consider this initially, but I was hoping to avoid it as it is a bit clunky to include the type in the function name, especially since I have two such types.
@Vasily_Pisarev thanks for the comment, this is among the solutions that I am still considering.
An improvement to your second method is to using Base: <thing> instead of import. The relevant difference being that defining a method for <thing> will not extend the Base method but will instead throw an error demanding that you use the qualified Base.<thing> to show you mean it. This should avoid accidental piracy, although you’d have to fix these errors as they arise in the future (probably quite rare, in practice).
From a technical perspective, you don’t need to worry about making sure that isopen is locally defined before you write it in a function. You just don’t want to call any of those functions (or directly evaluate isopen(...) itself) before that point. That’s general good practice for a module anyways — you don’t want to be running much code at top level, you just want to be defining it. This just gets muddled with interactive development at the REPL because there you’re flipping between execution and definition frequently and can easily accidentally call isopen before you’ve shadowed it. It looks like the v1.11 REPL makes this even more confused as it erroneously prevents shadowing Base’s exports with new function definitions. I think that’s a bug (edit: now julia#58342).
But yeah, you can just write:
module MyModule
# Lots of code definitions that use isopen...
include("file_that_defines_isopen.jl")
end
Thank you, I was more or less aware of this simple solution, but the problem is that it is vulnerable to certain relatively innocent code changing the meaning of code later on (e.g. if the “lots of code” includes a file that imports Base.isopen), so I wanted to do something that would help protect against such errors. I did actually work some more with @mikmoore s suggestion of using using rather than import and made a little package implementing this (and found another bug in my original post), but in the end, I was not very satisfied with this path and ended up just putting an assertion in the last line of the module and a comment explaining it:
module MyModule
# Lots of code
include("file_that_defines_isopen.jl")
# Explanation for the next line
@assert parentmodule(isopen) != Base
end
Another alternative could just have been to include this test in the testsuite for the package.
Thank you. I made an error in that post and corrected it in my next post. I just got it from the manual at Modules · The Julia Language , so it should work for 1.11 and be backwards compatible.