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

Print module correctly when extending a function #758

Closed
kellertuer opened this issue Jul 9, 2018 · 11 comments
Closed

Print module correctly when extending a function #758

kellertuer opened this issue Jul 9, 2018 · 11 comments

Comments

@kellertuer
Copy link

Let's say I want to extend the function definition of Base.LinAlg.norm in my new Package SuperNorms.jl and so I am writing somewhere in my package

import Base.LinAlg: norm
export norm

and define the new norm for ::MySuperType somewhere including a nice (super) doc string.

When I then generate the documentation and call

```@docs
norm
```

in my documentation.md the first rendered line (html-rendered by Documenter.jl ) reads

Base.LinAlg.normFunction
norm(s)
mySuperDocuText...

Why does it refer to the imported Module and not to SuperNorms.normFunction?
Is that a bug or do I have to change that somewhere?

For functions just defined in the package it works correctly.

@fredrikekre
Copy link
Member

It prints Base.LinAlg.norm because that is the function you extend, for not printing the module name, see #253

@kellertuer
Copy link
Author

Since the source link after that links to SuperNorms.jl#norm (i.e. the line where it's defined) – wouldn't it be nice to read SuperNorms.norm or at least state both, something like

SuperNorms.norm (extended from Base.LinAlg.norm) – Function?

Usually I would like to see the Module name – but with this last line, maybe in more detail than it now is.

@fredrikekre
Copy link
Member

If it reads SuperNorms.norm I would assume it was a different function than Base.LinAlg.norm.

@kellertuer
Copy link
Author

For me, just reading Base.LinAlg.norm always looks like Supernorms.jl was not that much involved in the norm function. What's really the case is, that it defines indeed a norm, but a completely different one, that just shares the name (it may eve even have three parameters required of special SuperNorm-Types) . Though it's also really a norm, hence the extension.

@fredrikekre
Copy link
Member

What's really the case is, that it defines indeed a norm, but a completely different one, that just shares the name (it may eve even have three parameters required of special SuperNorm-Types) . Though it's also really a norm, hence the extension.

IMO this is what the docstring should include, but it is important to show that it is a method of Base.LinAlg.norm that you document. Otherwise I would assume that I would have to use the function as SuperNorms.norm in my code.

@kellertuer
Copy link
Author

Now you leave me confused. I thought if I write a norm for a fancy new type of mine, but it is conceptually a norm and hence I would like to use the name of the function as norm (and for a few cases it even reduces to the implementation of Base.norm on my fancy types), but I do not want to override the function, i.e. since my types are completely different from those in Base, I'd like to extend it. Still the part I am defining is defined in SuperNorms (SuperNorms.norm) not in Base. For me the output of the first line indicates, that the function comes from Base.LinAlg.norm (completely). At least I am confused everytime to see Base.LinAlg there. The docstring of course properly states in its first line the signature, explains the Arguments, how it works and what it returns. So I'd prefer that line to read

SuperNorms.norm (extended from Base.LinAlg.norm) – Function

since it is an extension of the method of Base.LinAlg.norm that I document.

@mortenpi
Copy link
Member

I agree with @fredrikekre here. There is a difference between extending a function from another module and defining a new function with the same name as a function in another module. Displaying SuperNorm.norm for an overload implies the latter case, and would therefore be misleading.

We could potentially do it the other way around:

Base.LinAlg.norm (defined in SuperNorms) – Method.

@kellertuer
Copy link
Author

That sounds like a good solution to me; but yes my functions are norms, that's why I'd like to extend Base.LinAlg, but I'd also like to point out to the reader, that I extended it to y SuperNormTypes :)

@dwfmarchant
Copy link

Did anyone come up with a workaround for this one? I have a use case that I would really like to not display the module from which the function is being extended.

@kellertuer
Copy link
Author

Well since this is a feature and not a bug, I did not look for a workaround. In the long run it would maybe be nice to have both the origin and the new definitions module.

@dwfmarchant
Copy link

Thanks for the reply - I was able to get the behaviour I wanted by by making my own writer. I'll try and put up and example and maybe put together a pr some time soon.

@mortenpi mortenpi closed this as completed Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants