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

Re-export from LinearAlgebra and FillArrays #1368

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

torfjelde
Copy link
Contributor

Deprecations introduced in #1362 suggest alternatives that use the following:

  • LinearAlgebra.Diagonal
  • LinearAlgebra.I
  • FillArrays.Fill

It seems like these should then also be re-exported, in particular since Distributions.jl is a package that is often used by Julia beginners who might not be aware of where the above can be found.

@devmotion
Copy link
Member

I am a bit unsure about this PR. I agree that it might be more convenient since one can skip an using LinearAlgebra or using FillArrays but it seems it can become difficult to draw a line here. Should we reexport Zeros as well which is used in MvNormal? Or PDMat, ScalMat and PDiagMat which can be used in the constructor? So maybe it is more a documentation issue and should not be solved with reexporting I, Fill and Diagonal?

@torfjelde
Copy link
Contributor Author

Yeah that's a fair point 👍

One argument for exporting is that Distributions.jl has become a package that a lot users will interact with for the first time, and so making it completely trivial to use without any "surprises", e.g. examples not being completely "self-contained" in the sense that external dependencies are explicitly required, might be worth it.

But I leave this up to you all:)

@mschauer
Copy link
Member

mschauer commented Aug 27, 2021

I think we should not do it, it feels like working against the grain of Julia, after all

~$ exec '/Applications/Julia-1.6.app/Contents/Resources/julia/bin/julia'
julia> I
ERROR: UndefVarError: I not defined

seems to be a hurdle which has to be overcome in any case if someone wants to use Julia. But let's make sure the docs are helpful.

@ParadaCarleton
Copy link
Contributor

I will add a slight comment (which you can feel free to ignore) -- I personally had exactly this problem when working with Distributions.jl a couple months ago as a beginner in Julia (I didn't realize I is only part of LinearAlgebra).

@devmotion
Copy link
Member

I think we should qualify all functions, structs etc. from other packages in the docs, e.g., we should write LinearAlgebra.I or LinearAlgebra.Diagonal instead of I or Diagonal to make it clear where these objects are defined. IMO, though, we should not reexport Fill, Diagonal, I, PDMat etc.

@torfjelde
Copy link
Contributor Author

I think we should qualify all functions, structs etc. from other packages in the docs, e.g., we should write LinearAlgebra.I or LinearAlgebra.Diagonal instead of I or Diagonal to make it clear where these objects are defined. IMO, though, we should not reexport Fill, Diagonal, I, PDMat etc.

Sounds good 👍

@ghost
Copy link

ghost commented Oct 4, 2021

I like that idea.

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

Successfully merging this pull request may close these issues.

4 participants