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

Correctly mark all symbols as public or not #51335

Open
1 of 4 tasks
LilithHafner opened this issue Sep 15, 2023 · 22 comments
Open
1 of 4 tasks

Correctly mark all symbols as public or not #51335

LilithHafner opened this issue Sep 15, 2023 · 22 comments
Labels
domain:docs This change adds or pertains to documentation status:help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@LilithHafner
Copy link
Member

LilithHafner commented Sep 15, 2023

#50105 added a preliminary list of symbols to be marked as public from Base. We should finalize that list for Base, stdlibs, and submodules before 1.11.

Tasks

@LilithHafner LilithHafner added this to the 1.11 milestone Sep 15, 2023
@LilithHafner LilithHafner added the domain:docs This change adds or pertains to documentation label Sep 15, 2023
@LilithHafner LilithHafner mentioned this issue Sep 15, 2023
7 tasks
@LilithHafner LilithHafner added the good first issue Indicates a good issue for first-time contributors to Julia label Sep 21, 2023
@adityam003
Copy link

is this issue solved or can i help?

@LilithHafner
Copy link
Member Author

This issue is very much not solved, and we would appreciate your help!

Any symbol that is mentioned in the manual in Julia 1.9 or 1.10 and is not exported should be marked as public with public that_symbol. If you want to help, you can look through submodules of Base and/or standard libraries to find these symbols and mark them as public. Ideally, you would find many symbols and mark them all as public in a single PR so discussion can be more effective. In some cases it might not be clear whether or not a symbol is supposed to be public, feel free to take a guess and mention that you are unsure and other folks can weigh in with their opinions.

@adityam003
Copy link

Can you explain me bit more specifically about how should i approach this.

@LilithHafner
Copy link
Member Author

You could install Julia 1.11 (download the nightly or build from source) and then run names(Base.Iterators, all=true) or using Random; names(Random, all = true) or the same for any module. That will give a list of symbols. If you then run names(Base.iterators), that will give a list of public symbols. If you find things in the longer list that are explicitly mentioned the online manual, then they should probably also be in the shorter list and you can open a PR to mark them as public.

@adityam003
Copy link

so should i download all the dependencies for building julia or can i just clone it in my local machine and run for this task

@LilithHafner
Copy link
Member Author

Ideally, this should work:

$ git clone https://github.com/JuliaLang/julia
$ cd julia
$ make
... wait a long time ...
$ ./julia

If you try this and get an error and you are running on a mainstream OS, let us know, because that's probably a bug on our end.

@adityam003
Copy link

make doesnt work for windows

@LilithHafner
Copy link
Member Author

Ah, right. Windows. The steps for building from source are a bit more complicated: https://docs.julialang.org/en/v1/devdocs/build/windows/#Source-distribution

You don't have to build from source to contribute, but it can be very helpful in testing your PRs.

@nsajko
Copy link
Contributor

nsajko commented Oct 19, 2023

Base.include is documented in the manual and referred to from other help sections, so it seems like it's part of the public interface. The REPL help, however, says: "The following bindings may be internal; [...]"

@LilithHafner
Copy link
Member Author

How is this possible lol

julia> Base.include === include
false

@vtjnash
Copy link
Sponsor Member

vtjnash commented Oct 19, 2023

Every module has an include function that is private to that module

@nsajko
Copy link
Contributor

nsajko commented Oct 19, 2023

Base.include and Base.MainInclude.include are two different functions, both documented. The latter is the one exported from Base:

julia> Base.MainInclude.include === include === Main.include
true

@LilithHafner LilithHafner added status:help wanted Indicates that a maintainer wants help on an issue or pull request and removed good first issue Indicates a good issue for first-time contributors to Julia labels Nov 17, 2023
@inkydragon
Copy link
Sponsor Member

What about Stdlibs?

Take SHA.jl as an example.
All symbols list in docs are exported, so there is nothing to do, right?

docs HTML: https://docs.julialang.org/en/v1/stdlib/SHA/
docs md: https://github.com/JuliaCrypto/SHA.jl/blob/88e1c83c32790c1cfbd0d13cc71c7db32bd6a573/docs/src/index.md?plain=1#L61
export: https://github.com/JuliaCrypto/SHA.jl/blob/88e1c83c32790c1cfbd0d13cc71c7db32bd6a573/src/SHA.jl#L36-L49

Do we need to change export to public or something else?

@nsajko
Copy link
Contributor

nsajko commented Nov 19, 2023

Do we need to change export to public or something else?

No need to do that, AFAIK: either one of public or export denote public API, the difference is just that public doesn't pollute namespaces (when imported with using instead of import).

@ShrutiRDalvi
Copy link
Contributor

Hello,

Is this issue solved, else I'd love to contribute. It is my first time contributing to open source, so any tips on how to go about it would help a ton.

Thanks

@lgoettgens
Copy link
Contributor

lgoettgens commented Feb 7, 2024

For the Tar stdlib, there are JuliaIO/Tar.jl#174 and JuliaIO/Tar.jl#175.

@matthias314
Copy link
Contributor

I'm looking at candidates in Base.Broadcast. Where should the corresponding public statement go? To broadcast.jl (following export?), to public.jl or elsewhere?

@mcabbott
Copy link
Contributor

Xref #53900 for Base.Iterators.

For Base.Broadcast I believe they would go in https://github.com/JuliaLang/julia/blob/master/base/broadcast.jl

@matthias314
Copy link
Contributor

matthias314 commented Apr 12, 2024

OK, the PR for Base.Broadcast is #54060.

@KristofferC
Copy link
Sponsor Member

I don't think this has to be on the milestone since adding a public can always be done in a later release.

@KristofferC KristofferC removed this from the 1.11 milestone Apr 23, 2024
@LilithHafner
Copy link
Member Author

It's just that users will start getting "this symbol may be internal" warnings for docstrings of nonpublic symbols in 1.11

@nsajko
Copy link
Contributor

nsajko commented May 3, 2024

Base.dataids should be public, right? It's doc string says:

Custom arrays that would like to opt-in to aliasing detection of their component parts can specialize this method to return the concatenation of the dataids of their component parts. A typical definition for an array that wraps a parent is Base.dataids(C::CustomArray) = dataids(C.parent).

But see #50820 #51753

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:docs This change adds or pertains to documentation status:help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Development

No branches or pull requests

10 participants