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

Use leading underscores everywhere on internal functions #94

Open
rafaqz opened this issue Aug 27, 2024 · 9 comments
Open

Use leading underscores everywhere on internal functions #94

rafaqz opened this issue Aug 27, 2024 · 9 comments

Comments

@rafaqz
Copy link
Member

rafaqz commented Aug 27, 2024

Even with the new public keywords we need to make internals look scary, the consequences of changing them is too large now.

See #92

(Although doing this now ironically risks massive ecosystem breakage, so maybe just with new functions...)

@rafaqz rafaqz changed the title Use underscores everywhere on internals Use leading underscores everywhere on internal functions Aug 27, 2024
@jw3126
Copy link
Member

jw3126 commented Aug 28, 2024

I don't think it is our responsibility to prefix every private function. If we really want to make private stuff private, we could use the following pattern:

module ConstructionBase

function setproperties end
function getproperties end
...

module _Private
import ..ConstructionBase: setproperties, getproperties, ...

include("some.jl")
include("files.jl")
...
end
end

@rafaqz
Copy link
Member Author

rafaqz commented Aug 28, 2024

I'm not sure I see the reasoning, underscores are less fuss and everyone knows what they mean.

And most of my packages use underscores and a lot of Base Julia does, it's not an unusual thing to do.

@jw3126
Copy link
Member

jw3126 commented Aug 28, 2024

Sure, it is about the "everywhere" part. Typically some internal functions are prefixed with _ but it requires a lot of discipline to do it with every private function. While I have often seen some internals being prefixed, I have never seen it being done fully consistently.

@rafaqz
Copy link
Member Author

rafaqz commented Aug 28, 2024

I'm just thinking about the relative scale of that discipline vs the massive ecosystem outages confusion about our methods can cause. (to clarify, I mean just putting underscores is a tiny effort vs other package devs frantically patching versions to make hundreds of packages work)

@jw3126
Copy link
Member

jw3126 commented Aug 28, 2024

I'm just thinking about the relative scale of that discipline vs the massive ecosystem outages confusion about our methods can cause.

Prefixing every function with _ vs putting them into a module both cause the same outage. For backwards compat you can do:

module ConstructionBase
function getproperties end
function setproperties end
...
# Internal functions that are de facto used by other packages however
# we keep them here to not break these packages
function setproperties_object end
...

@rafaqz
Copy link
Member Author

rafaqz commented Aug 28, 2024

Yes you can do the same thing either way. I'm just not sure why we would do something different to what Base julia and most people do, which is use underscores. But if you really want separate modules sure.

The idea of this post was to respond constructively to breaking half of the SciML ecosystem.

@jw3126
Copy link
Member

jw3126 commented Aug 28, 2024

Yes you can do the same thing either way. I'm just not sure why we would do something different to what Base julia and most people do, which is use underscores.

I read your opening post as a higher bar than what base julia or other packages do. Not all internals of Base are prefixed, internals of Base get used all the time and nightly breaks some ecosystem every day or so.

My understanding was you want to be fully rigorous about what is private. In this scenario my suggestion was to
enforce it through code rather than rely on discipline.

@jw3126
Copy link
Member

jw3126 commented Aug 28, 2024

Anyway if you make a PR that adds underscores everywhere I am not going to object.

@rafaqz
Copy link
Member Author

rafaqz commented Aug 28, 2024

This package is very small, so the ratio of dependencies to lines of code is actually much higher than Base Julia! So I would argue effort/reward for things like this is higher here.

See when I have time for the PR, but I think we should also deprecate the non-underscore functions so people get a warning to stop using them.

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

No branches or pull requests

2 participants