Fastai: Weird syntax for symmetric_warp

[RohitMidha23]

I'm not sure if this is how it's supposed to be but this sure does seem weird for syntax.
This screenshot has been taken from the fast.ai website from Chrome browser.

I would be happy to fix it if someone could point out where the doc for this is generated.

[stas00]

Thank you for reporting this documentation problem, @RohitMidha23.

It looks like a bug in the doc converter. It affects at least:

• https://docs.fast.ai/vision.transform.html#_symmetric_warp
• https://docs.fast.ai/vision.transform.html#_perspective_warp

@bearpelican, can you please have a look? The source nb looks correct, but once it's converted it gets messed up.

It looks like it can't handle the extra key=val inside partial type - e.g. , it works without the key in _dihedral_affine below, but not in _perspective_warp that has the arg name:

def _dihedral_affine(k:partial(uniform_int,0,7)):
def _perspective_warp(c, magnitude:partial(uniform,size=8)=0, invert=False):

[RohitMidha23]

Can I work on this issue? First time trying it out but I would love to try!

[stas00]

Yes, of course, @RohitMidha23.

First you need to set things up: https://docs.fast.ai/gen_doc_main.html#process-for-contributing-to-the-docs

Then learn how to build the docs: https://docs.fast.ai/gen_doc_main.html#building-the-documentation-website

Then you can look at the code under fastai/gen_doc/ to identify where things go wrong in the doc generator.

@bearpelican developed most of it, so you may have to ask him questions if you get stuck and he is up for helping you and not just fixing it himself.

[RohitMidha23]

That's amazing!
Thanks for linking me to the resources. I'll get started ASAP.

[RohitMidha23]

@stas00 from what I was able to gather the syntax in the metadata of the notebook should look like :

> <code>symmetric_warp</code>(**`c`**, **`magnitude`**:<code>partial(</code>[`uniform`](/torch_core.html#uniform), `k=`4<code>)</code>=**`0`**, **`invert`**=**`False`**) → [`Image`](/vision.image.html#Image) :: [`TfmCoord`](/vision.image.html#TfmCoord)\n 

to generate :

symmetric_warp(c, magnitude:partial(uniform, k=4)=0, invert=False) → Image :: TfmCoord\n

But the notebook metadata currently is :

> <code>symmetric_warp</code>(**`c`**, **`magnitude`**:<code>partial(</code>[`uniform`](/torch_core.html#uniform), `k=`4``<code>)</code>=***`0`***, **`invert`**=***`False`***) → [`Image`](/vision.image.html#Image) :: [`TfmCoord`](/vision.image.html#TfmCoord)\n

which generates the issue.

Working on finding the function that does this now. @bearpelican if you can point me to that it'll be massively helpful!

[stas00]

Yes, an extraneous `` and 2 ***:

***`0`***
***`False`***

I have no idea how it came to be. @bearpelican is the magician in charge! ;)

The general area is fastai/gen_doc/nbdoc.py, e.g. format_param generates those quoted above.

[RohitMidha23]

@stas00 I'm able to get rid of the *. Minor change in format_param, yes.
If we remove code_esc() from here to make it just if include_bt: arg_name = arg_name, it works, not really sure why. However that will make changes in a lot of places. @bearpelican Can you suggest a work around?

[stas00]

@stas00 I'm able to get rid of the *. Minor change in format_param, yes.

careful that other things don't break though.

I think that *** is there by design, but I don't know why. Is it ** + * bold+italic?
It was added in this PR: https://github.com/fastai/fastai/pull/1515 fixing the backticks which weren't getting rendered.

Also we should probably stick to a clean markdown so also replacing <code> with ` everywhere: e.g now we have a mix of both: <code>symmetric_warp</code>(**c**, **magnitude**:<code>partial(</code>[

[RohitMidha23]

I think that *** is there by design, but I don't know why. Is it ** + * bold+italic?
It was added in this PR: #1515 fixing the backticks which weren't getting rendered.

@stas00 you're right, I hadn't thought of that. I've fixed it in that case also. The only other problem now would be the extra `` .

Also we should probably stick to a clean markdown so also replacing <code> with everywhere: e.g now we have a mix of both: symmetric_warp(c, magnitude:partial([`

I'm not sure why but I think the <code> was used for a specific reason. Maybe @bearpelican can clarify more on that.

[RohitMidha23]

@stas00 I figured it out to an extent.
In this line here we call link_type(v). For perspective_warp, size = 8 is the second argument. So when we call link_type(v) it gives 8 back as 8. And now in that line, k = 8. The problem arises when we call link_type(v) again in the next line. This returns it with an extra `.
Therein lies our problem.
Further what has been missed out is that k is actually size and hence we should be calling link_type for k too. Thereby making this line :

args = (t.func,) + t.args + tuple([f'{link_type(k)}={link_type(v)}' for k,v in t.keywords.items()])

However by following the logic above we get a pair of extra ` which surround the =.
Hence our output looks like :

With the current code, the simplest solution would be to change this line to :

args = (t.func,) + t.args + tuple([f'{k}={v}' for k,v in t.keywords.items()])

Hence our output would look like :

[stas00]

Without looking at the outcome of this proposed change, if you are removing link_type function call, most likely it's going to affect every other situation where there are no nested default values - which is 99% of cases. Perhaps the code could detect that it's a nested definition and encode it as an independent string?

I suggest that while you fix these 2 oddities, you pay close attention that other functions' md/html docstrings don't get impacted.

[RohitMidha23]

@stas00 again, you're right. I was just looking into that.

[RohitMidha23]

@stas00 correct me if I'm wrong and I know this is slightly naive but the function partial_repr is called only when the object is of instance partial as can be seen from this line.

The thing to note is that only 5 functions use partial in their methods. All of these functions have integer arguments as their second and third arguments and hence the function link_type is of no use for this particular function, from my understanding.

When I changed the code in nbdoc as I proposed above and built the docs, the nbdiff showed only two changes, for perspective_warp and symmetric_warp.

The output of nbdiff I got was :

[stas00]

Yes, you found a bug - it should have been {k} and not k! Good one, @RohitMidha23

And the rest of your analysis is correct.

So what you're proposing is this:

diff --git a/fastai/gen_doc/nbdoc.py b/fastai/gen_doc/nbdoc.py
index 1876e4ba..5daf55bb 100644
--- a/fastai/gen_doc/nbdoc.py
+++ b/fastai/gen_doc/nbdoc.py
@@ -59,7 +59,7 @@ def type_repr(t):
     else: return link_type(t)

 def partial_repr(t):
-    args = (t.func,) + t.args + tuple([f'k={link_type(v)}' for k,v in t.keywords.items()])
+    args = (t.func,) + t.args + tuple([f'{k}={v}' for k,v in t.keywords.items()])
     reprs = ', '.join([link_type(o) for o in args])
     return f'<code>partial(</code>{reprs}<code>)</code>'

I rebuilt all nbs and indeed only these 2 entries got modified and they look correct now.

Excellent job, @RohitMidha23.

So next please submit a PR for that code https://docs.fast.ai/dev/git.html#how-to-make-a-pull-request-pr and we will merge it and fix the broken docs automatically - no need to change the docs in your PR that is.