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

Minor documentation clarification issues with arrays #13427

Closed
ScottPJones opened this issue Oct 2, 2015 · 2 comments
Closed

Minor documentation clarification issues with arrays #13427

ScottPJones opened this issue Oct 2, 2015 · 2 comments
Labels
docs This change adds or pertains to documentation

Comments

@ScottPJones
Copy link
Contributor

There seem to be some minor issues where the code and the documentation are not consistent,
so before submitting a PR to fix them, I'd like to find out which is actually correct.

Base.linearindexing{T<:MyArray}(::Type{T}) = LinearFast()

Isn't this the same as writing:
Base.linearindexing(::Type{MyArray}) = LinearFast()?

The sparse() function is often a handy way to construct sparse matrices. It takes as its input a vector I of row indices, a vector J of column indices, and a vector V of nonzero values.

I believe this is incorrect, because in the code, it actually removes any zero values that are in the vector V. Vector V is not restricted to nonzero values.

One final issue is that there is some documentation about the different types of sort, which is only present in one variant sort! (i.e. the description of which algorithm is used, the meaning of the by and lt keyword arguments and how they interact).
In the manual, it makes sense for that information to be only present once, however, for the help information from doc or ?, they at least need a line "See also sort!" added.
I'm not sure if there is any way to handle that cleanly with the new doc system.

@mbauman
Copy link
Member

mbauman commented Oct 3, 2015

Isn't this the same as writing:
Base.linearindexing(::Type{MyArray}) = LinearFast()?

No. That is very specifically written that way because array subtypes almost always have type parameters. In those cases, writing it as you have would be wrong, because invariance.

vector V of nonzero values

I'd call that ambiguous, not incorrect. A small clarification would be fine. As are more see also sections. There are already quite a few; just arrange your additions to match the existing style.

[Edit: removed aside]

@ScottPJones
Copy link
Contributor Author

No. That is very specifically written that way because array subtypes almost always have type parameters. In those cases, writing it as you have would be wrong, because invariance.

OK, thanks for the info. I do think that is something easy to miss (I've seen a questions about it on julia-users), do you think it deserves maybe an explanation as to why the difference here is critical?

I disagree that saying "vector V of nonzero values" is ambiguous - it seems to be stating pretty clearly that the values should only be non-zero, and there is no indication of what would happen if
zero values are in fact present.
It is also important to clarify the handling of zeros present in vector V, since otherwise, one might be led to believe that they are handled as indicated in the following section:

In some applications, it is convenient to store explicit zero values in a SparseMatrixCSC. These are accepted by functions in Base (but there is no guarantee that they will be preserved in mutating operations). Such explicitly stored zeros are treated as structural nonzeros by many routines.

[Edit: removed response to aside]

@ScottPJones ScottPJones changed the title Documentation issues with arrays Minor documentation clarification issues with arrays Oct 3, 2015
@JeffBezanson JeffBezanson added the docs This change adds or pertains to documentation label Oct 23, 2015
tkelman pushed a commit that referenced this issue Jun 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

No branches or pull requests

3 participants