Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some sort of policy to regarding unexported functions in Base? #12647

Closed
IainNZ opened this issue Aug 16, 2015 · 99 comments
Closed

Some sort of policy to regarding unexported functions in Base? #12647

IainNZ opened this issue Aug 16, 2015 · 99 comments
Labels
julep Julia Enhancement Proposal

Comments

@IainNZ
Copy link
Member

IainNZ commented Aug 16, 2015

There have been a couple of times during 0.4 development where unexported functions have been removed or changed. Every time its come up we seem to go in circles a bit about what to do about it. Given the resources available, I feel we should just keep it simple:

No support or deprecation warnings, just change/remove them. Only exported functions get deprecation warnings or mitigation regarding changing arguments.

But we need consensus around this to make it stick.

@timholy
Copy link
Member

timholy commented Aug 16, 2015

Even though I've paid the price for this myself on occasion, I can't imagine any other sensible policy. 👍

@johnmyleswhite
Copy link
Member

+1

1 similar comment
@rsrock
Copy link
Member

rsrock commented Aug 16, 2015

+1

@IainNZ IainNZ added the julep Julia Enhancement Proposal label Aug 16, 2015
@ScottPJones
Copy link
Contributor

Unexported things are definitely supported in Julia, and are deprecated all the time.
Things like to_index and compilecache are not exported, for example.
compilecache had been called compile. If that had made it into 0.4, instead of being changed before release, it would have needed to be deprecated as well.
The real issue is that Julia doesn't have any way currently of declaring that a function/method/type should not be accessible outside of it's module, which would have prevented these cases of internal functions being used outside base, and there are other valid reasons for not exporting a name (to avoid name pollution).

@timholy
Copy link
Member

timholy commented Aug 16, 2015

In general user-code should not call to_index or compilecache. __precompile__ is an exported function.

@kmsquire
Copy link
Member

A very minor example of a function that would be affected by this policy is Base.permute!!(a, p). It exists so that Base.permute!(a, p) can permute a in place, but leave the permutation p untouched. But if a user knows that p is okay to clobber, she may wish to call the double-bang version, to avoid copying p.

It's unexported and undocumented, and therefore not widely used outside of Base (only DataFrames that I'm aware of, probably because I added it there). It's also the only example I'm aware of in base with a '!!', which is probably why there was resistance to export it in the first place (ref: #2201).

I don't think this is anything that should prevent the policy proposed here from being put in place (these things should probably just be handled on a case-by-case basis as needed), but I thought I would mention it.

@aviks
Copy link
Member

aviks commented Aug 16, 2015

I am agnostic about this being a policy in Base . However, I think we should be careful in claiming this to be an overall Julia design guideline. (Not that anybody is suggesting that at the moment, but people tend to follow practices from base, and so I thought I should articulate this, if only for the record)

Given Julia's module features, it is sometimes good design, and at other times necessary, to not export names from some module. Every package can, and needs to, make this decision on its own.

@ivarne
Copy link
Member

ivarne commented Aug 16, 2015

I think @aviks is right, and that there should be room for documenting (and deprecating) unexported names (especially in packages, but it also makes some kind of sense in Base).

Maybe it would make sense to tie deprecation/midigation policy to documentation status, instead of only exportation status?

@IainNZ IainNZ changed the title Some sort of policy to regading unexported functions? Some sort of policy to regarding unexported functions in Base? Aug 16, 2015
@IainNZ
Copy link
Member Author

IainNZ commented Aug 16, 2015

Updated title to keep this focussed on Base only, I don't want to get into packages (they can do whatever the feel is appropriate).

@mauro3
Copy link
Contributor

mauro3 commented Aug 16, 2015

One counterexample to this proposal in Base is the Profile module which only exports @profile. But quite a few methods belong to its public interface, for instance Profile.print.

I think a way to specify which unexported functions belong to the public interface would be good. Which is similar @ScottPJones's comment above about making methods inaccessible (although I wouldn't go that far).

@toivoh
Copy link
Contributor

toivoh commented Aug 16, 2015 via email

@tknopp
Copy link
Contributor

tknopp commented Aug 16, 2015

Why aren't these functions in the Profile module exported?

My mental model was that public==exported which seem to hold in 99% of the cases except where submodules are used for introducing namespaces

@timholy
Copy link
Member

timholy commented Aug 16, 2015

Profile does introduce a namespace. Notice the functions are called print, init, clear, and retrieve; odds are quite high that you don't want Profile.print instead of Base.print each time you say print. Given that we have modules, there's no reason to call them profile_print, profile_init, and profile_retrieve. Pkg behaves by similar rules.

@alyst
Copy link
Contributor

alyst commented Aug 16, 2015

Just a thought from a user perspective.
I think in general it would be nice to let power users/package developers to take advantage from some "advanced" Base functions (e.g. permute!!()) and commit to tracking their compatibility between the releases.
Such functions are not meant to be called from the REPL, so they could be safely left unexported. However, they still need to be a part of Julia documentation (i.e. have a @doc block) to be properly used. That could be their distinctive feature.
If somebody wants to use something Julia developers have not documented, she/he should be really on her/his own.

@aviks
Copy link
Member

aviks commented Aug 16, 2015

Given that we have modules, there's no reason to call them profile_print, profile_init, and profile_retrieve. Pkg behaves by similar rules.

This is the kind of use I was referring to. I was thingking of JSON.write() but there clearly are lots of such uses in Base.

My mental model was that public==exported ...

I don't think that can or should be true in Julia.

But let me say, while I am debating the larger design problem, as far as the specifics of this issue goes, I am all for nuking code aggressively. Julia is still a 0.x language. If it looks like some code is un/under-used, delete it, with a good faith effort at notifying packages. At this stage in our lifecycle, that is perfectly acceptable, nay, necessary. Also (to prevent some counterarguments), I say this as someone who uses Julia in real life :).

@tkelman
Copy link
Contributor

tkelman commented Aug 16, 2015

I don't like making this a policy, as phrased it seems to be encouraging introducing breaking changes more often without any migration options. Yes it's risky to depend on unexported functionality from base, we should write that down in some form and let it be known, and be more careful about only doing so when absolutely necessary in packages. And we need to be able to detect and highlight more clearly when it's happening, so we know about it in advance. Is this one of the things Lint.jl can or does check for? Have many established packages started using Lint.jl in their development process the way CI and coverage tools are being used?

We shouldn't be introducing frivolous breakage, even in unexported functions, without good reasons for doing so (there are plenty of good reasons as the language develops, but that's a difficult line to draw in policy terms). There are enough cases where using unexported functionality is being done and necessary, that "break things at will" as a written policy means things will be broken more often. Until we have the hardware resources to run PackageEvaluator reliably, 100% daily, and/or on pre-merged PR's, this makes working against -dev versions of Julia even less stable. We've done a really lousy job of keeping users away from -dev versions. Turns out people like new features, and releases are taking time. When users are the ones finding breakage in packages before the package developers do, and are just reporting issues instead of helping fix the problems with PR's, that's a sign that too many people are on -dev. And that we're not doing a good job of teaching enough people how to participate in fixing the bugs they find when using unstable versions of Julia.

As I see it, base has more developer resources than packages. Less effort here for more breakage in packages doesn't seem like a good tradeoff, unless we manage to change the onboarding process of who uses -dev versions of Julia and how breaking changes get fixed across the ecosystem.

@StefanKarpinski
Copy link
Member

I think the issue here may be that we need separate mechanisms for expressing that something is public and expressing that it should be exported (in the current sense).

On Aug 16, 2015, at 7:48 PM, Tony Kelman [email protected] wrote:

I don't like making this a policy, as phrased it seems to be encouraging introducing breaking changes more often without any migration options. Yes it's risky to depend on unexported functionality from base, we should write that down in some form and let it be known, and be more careful about only doing so when absolutely necessary in packages. And we need to be able to detect and highlight more clearly when it's happening, so we know about it in advance. Is this one of the things Lint.jl can or does check for? Have many established packages started using Lint.jl in their development process the way CI and coverage tools are being used?

We shouldn't be introducing frivolous breakage, even in unexported functions, without good reasons for doing so (there are plenty of good reasons as the language develops, but that's a difficult line to draw in policy terms). There are enough cases where using unexported functionality is being done and necessary, that "break things at will" as a written policy means things will be broken more often. Until we have the hardware resources to run PackageEvaluator reliably, 100% daily, and/or on pre-merged PR's, this makes working against -dev versions of Julia even less stable. We've done a really lousy job of keeping users away from -dev versions. Turns out people like new features, and releases are taking time. When users are the ones finding breakage in packages before the package developers do, and are just reporting issues instead of helping fix the problems with PR's, that's a sign that too many people are on -dev. And that we're not doing a good job of teaching enough people ho w to participate in fixing the bugs they find when using unstable versions of Julia.

As I see it, base has more developer resources than packages. Less effort here for more breakage in packages doesn't seem like a good tradeoff, unless we manage to change the onboarding process of who uses -dev versions of Julia and how breaking changes get fixed across the ecosystem.


Reply to this email directly or view it on GitHub.

@timholy
Copy link
Member

timholy commented Aug 17, 2015

Yep. Currently, the best indication is: is the function documented? If not, it's likely not intended as part of the public API. All the exceptions raised so far are in that camp.

@ScottPJones
Copy link
Contributor

Even things that are not intended to be part of the public API should have internal documentation though,
just so that somebody besides the original author (or the author years later) can figure out what is going on.

@timholy
Copy link
Member

timholy commented Aug 17, 2015

devdocs

@ScottPJones
Copy link
Contributor

Having devdocs is very good, but is not a replacement for having internal documentation of functions.
If you have to deal with code whose lifespan is measured in decades, it is rather important.
I'd like to think that Julia will still be relevant, being updated and supported in 30 years from now.

@hayd
Copy link
Member

hayd commented Aug 17, 2015

Perhaps help could say that the method is not exported?

It's impossible to support every internal method change between dev versions (e.g. 0.4-dev) but perhaps it would be worthwhile comparing major versions. i.e. upon 0.4.0 what internal methods have changed or been removed since 0.3.

@ScottPJones
Copy link
Contributor

I think the issue here may be that we need separate mechanisms for expressing that something is public and expressing that it should be exported (in the current sense).

This is really not essentially different from what I've been suggesting, except that it is opt-in rather than opt-out (of being a public, not exported function/method/type).
Doing it opt-in, as @StefanKarpinski suggested, is probably better, since it seems that it is less common in Julia to have things public but not exported, than to have things not exported with the expectation that they are "private" in some sense.

@hayd It would be a good idea of having help display that, yes, esp. if there is a way of marking something as part of the public API, but not exported.
No, you wouldn't want to try to compare internal method changes, the real problem right now is that you can't distinguish between what is really internal and what really is part of the public API.

@tknopp
Copy link
Contributor

tknopp commented Aug 17, 2015

@StefanKarpinski, @JeffBezanson: Could you clarify if the intention of export was that exported functions form the public interface while unexported functions are private? We see that there are currently exceptions (submodules like Pkg and Profile) but it before putting to much weight on the exceptions it would be interested what the purpose of export was/is.

@StefanKarpinski
Copy link
Member

export controls what is available via using – that's it at the moment. It's an open question whether we need a different notion of public/private than that. One idea is to separate the concept of "exported" from "public": exported things are made available by using whereas public names still need qualification but are visible; the implication would be that anything not public would no longer be accessible even with qualification. Of course, you would still be able to sneak around that restriction using eval, but it would make it much more obvious that you were, in fact, doing something sneaky. It would probably make sense to allow non-const public bindings to be written to externally as well. So that would mean you'd have this kind of thing:

module Foo
    xx = 0
    const ro = 1
    public rw = 2
    public rwi::Int = 3
    usable = 4
    export usable
end    

using Foo

xx                  # not defined
ro                  # not defined
rw                  # not defined
rwi                 # not defined
usable => 4

Foo.xx              # not allowed
Foo.ro => 1
Foo.rw => 2
Foo.rwi => 3
Foo.usable => 4

Foo.xx = -1         # not allowed
Foo.ro = -1         # not allowed
Foo.rw = -1
Foo.rwi = -1
Foo.usable = -1     # ???

Foo.xx = "zz"       # not allowed
Foo.ro = "zz"       # not allowed
Foo.rw = "zz"
Foo.rwi = "zz"      # not allowed
Foo.usable = "zz"   # ???

Having export and public be orthogonal may make sense. This requires some thought.

@tknopp
Copy link
Contributor

tknopp commented Aug 17, 2015

I know that export is currently about scoping. In my opinion public and export are not orthogonal and it would complicate the system to much. We IMHO need a single mechanism to say what the (public) interface of a module is.

Can't the submodule use case be solved differently? I.e. the submodule exports the function and the mother module does not reexport the individual functions but only the scoped ones?

@mauro3
Copy link
Contributor

mauro3 commented Aug 17, 2015

@tknopp: your last suggestion does not work in the case of Base.Profile.print as exporting print would clash with Base.print. Renaming it could work though.

@ScottPJones
Copy link
Contributor

I believe most people working with Julia expects (and desire) rapid development with the trade off that things might have to break from time to time.

I heard quite a bit of grumbling at JuliaCon about breakages. I think the issue is really, what can be done to minimize breakage, while still allowing rapid development of the language.
Compat.jl and the deprecations are good examples of things that do help that goal, and I think that being able to make a clear distinction between what the author believes to be part of the public API or not also would help that goal.

@mbauman
Copy link
Member

mbauman commented Aug 19, 2015

This is getting all jumbled up with other issues.

I like Iain's initial suggestion — non-exported functions are officially not supported unless otherwise documented — with some of Tony's caveats about avoiding gratuitous breakages. As someone who has added a deprecation for a non-exported method, I'd add that I did so because in that case it was really easy… but I definitely don't want to set a precedent that all internal methods should be deprecated. If we notice that an internal function is getting used often, we should identify why and work on moving it towards a public API.

It'd be very interesting if we could search through packages and identify uses of internal functions. To make that work, though, we do need some mechanism for making a name public but not exported to support things like Profile.print(). Perhaps it's as simple as having documentation in the __META__ dict.

@tknopp
Copy link
Contributor

tknopp commented Aug 19, 2015

I don't think this is enough. We need a way to address the submodule case in order to write a function public_interface(m::Module). Since internal functions also can have documentation it is too indirect to go through __META__.

@mbauman
Copy link
Member

mbauman commented Aug 19, 2015

Since internal functions also can have documentation it is too indirect to go through __META__.

I agree that going through __META__ is a little indirect, but I think that, at least in Base Julia, only public functions should have docstrings. Internal methods can get comments in the code. I've been missing apropos since the Doc switchover, and a reasonable way for it to be re-implemented is to search through all the values of the __META__ dictionaries. If internal methods began showing up there it'd make the situation worse.

@ScottPJones
Copy link
Contributor

I disagree strongly about internal methods not being able to have docstrings. I might have fairly complex internal methods, that I want to have real documentation, and annotate things like what exceptions they throw, etc.

@tknopp
Copy link
Contributor

tknopp commented Aug 19, 2015

@mbauman: True. But these private docstrings could be accessible through some other functions. Thats why I think it would be better to have a direct way to define the interface of a module (i.e. export + some clever improvement to let Print.print go through some sort of export). If you have public_interface(m::Module) it could be used in apropos for filtering the actual API.

@ScottPJones
Copy link
Contributor

@tknopp, I think private docstrings should be accessible from help, but maybe by doing something like doing two ?. About export, I agree with Stefan, that the name export doesn't make as much sense anymore, so I would advocate public as a preferred alias, i.e. public Print.print, foobar, blech, exactly as has been suggested for extending export.

@timholy
Copy link
Member

timholy commented Aug 19, 2015

@ScottPJones, I suspect you're in the minority about wanting internal methods to have docstrings. That's what code comments are for.

@ScottPJones
Copy link
Contributor

When you are programming / debugging on a large project, you'd like to be able to have help come up, and not have to go digging into the source code all the time. Code comments simply don't cut it.

@toivoh
Copy link
Contributor

toivoh commented Aug 19, 2015 via email

@timholy
Copy link
Member

timholy commented Aug 19, 2015

And we go around in circles again. There are two camps: people who want to make internal functions easier to use (so they should have docstrings), and people who want to make sure our API is so nice that there's never any need to call an internal function. I'm in the latter camp. The absence of docstrings for internal functions is a way of forcing us to get the API right.

Code comments are just fine for documenting something whose usage you actively want to discourage.

@tknopp
Copy link
Contributor

tknopp commented Aug 19, 2015

Tim you are not alone. It would be pretty strange if our public API documentation would include documentation of internal functions. I don't care if there is some help_internal function though.

@ScottPJones
Copy link
Contributor

@timholy You are forgetting the people who have to develop the software that uses those internal functions. I'm not trying to make internal functions easier to use outside of the module they are in, rather make it so it is clear they are internal, which doesn't happen now, but also, to make it easier for the people maintaining the package or module, or for the poor programmer that gets a backtrace with that internal function, who'd like to quickly find out what it is supposed to be doing. That's why I thought a ?? at the REPL, which could call a help_internal, would be nice.

@mbauman
Copy link
Member

mbauman commented Aug 19, 2015

We have @less, @edit, etc. We're going down yet another rabbit hole here. I only brought up docstrings because they seem to be an easy and straightforward way to define the public interface by convention right now, without adding any new language features.

@StefanKarpinski
Copy link
Member

This discussion is getting nowhere quickly and isn't going to need to be settled for several months. How about we all let this sink in and simmer for a bit and revisit the issue when it's more pertinent.

@rsrock
Copy link
Member

rsrock commented Aug 19, 2015

@StefanKarpinski I just finished some doc edits (two minutes ago) that attempt to lay out how things currently work. It could either help frame the discussion, or it could get the pot boiling again.

Shall I hold off on the PR?

@StefanKarpinski
Copy link
Member

No, go for it – improvements to the documentation of how things currently work are always good.

rsrock added a commit to rsrock/julia that referenced this issue Aug 19, 2015
Fixes JuliaLang#12647

Explains the public nature of Julia names, and how to obtain some measure of
privacy when needed.  Also provides guidance for breaking changes in public APIs,
unexported names, and private (underscore) names.

[ci skip]
@rsrock
Copy link
Member

rsrock commented Aug 19, 2015

Ok, it's in #12696

@aviks
Copy link
Member

aviks commented Aug 19, 2015

That is a good advice @StefanKarpinski . But allow me to make a meta point

The issues raised here go the heart of what kind of a language Julia is. Is it a permissive language or a restrictive one. Is it a language that makes many weird things possible, or a language that prevents many bad things happening...etc.. etc... (There is no value judgement in either of these, nor are they binary.. but decisions like this go a long way to define the feel of a language)

For things like this, I think any movement towards "design-by-committee" are dangerous. Which does not prevent me from expressing an opinion, not should it, for anybody. I'm sure they all are useful input. But we should all be wary of all our wishes coming true.

rsrock added a commit to rsrock/julia that referenced this issue Aug 19, 2015
Fixes JuliaLang#12647

Explains the public nature of Julia names.  Also provides guidance for
breaking changes in public APIs vs. unexported names.

[ci skip]
@tknopp
Copy link
Contributor

tknopp commented Aug 20, 2015

@aviks: These are absolutely great points and if one follows the issue tracker and the mailing list you can see that the core maintainers see Julia as a permissive language. One issue is that there are sometimes newcomers having a different background and a very strong opinion which is expressed in verbose form so that one has to read these issues carefully to understand which is the "spirit" of Julia.

@ScottPJones
Copy link
Contributor

@tknopp One can still have the language be permissive, and at the same time add support for programmers to express their intentions, and help make things more reliable.
As @aviks said, it is not a binary issue - the language can be both permissive, and yet have mechanisms that be used to help prevent bad things from happening.

@tpapp
Copy link
Contributor

tpapp commented Apr 8, 2016

In the meantime, until this issue is fixed, maybe it would make sense to standardize on some sort of style convention for functions which are not part of an API, which could even go in the style guide.

A suggestion from existing practice: many libraries use a _ prefix for "internal" functions, and for the packages I have installed,

$ grep -ro 'function _' ~/.julia/v0.4 | wc -l
752

If eg .! or some similar syntax is introduced, uses of SomeModule._hey_you_shouldnt_use_this() can easily be found and replaced. If this issue is never resolved, at least we have a convention that is orthogonal to docstrings and other issues.

@IainNZ
Copy link
Member Author

IainNZ commented Jun 2, 2018

Tidying up old issues/PRs.

@IainNZ IainNZ closed this as completed Jun 2, 2018
@tpapp
Copy link
Contributor

tpapp commented Jun 3, 2018

@IainNZ: how was this issue resolved?

@IainNZ
Copy link
Member Author

IainNZ commented Jun 3, 2018

I believe indirectly its been resolved by there being a 1.0 soon, which has (or will have) guarantees. There wasn't much value to be had from this years-old discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
julep Julia Enhancement Proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.