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

[WIP] Added API link in examples #1013

Merged
merged 19 commits into from
Jan 26, 2020
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions doc/sphinxext/gallery_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def execfile(filename, globals=None, locals=None):
.. image:: {img_file}

**Python source code:** :download:`[download source: {fname}]<{fname}>`
**API documentation:** `{api_name} <../../generated/arviz.{api_name}>`_
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could be changed to {api_text} so that below, if api_name is None nothing is written?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OriolAbril , I have done the changes, now it'll show None <None> and not the directory link.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more clear replacing the whole line, so that when no match is fount (e.g. styles case) instead of **API doc... a white line is written. Or alternatively something like:

API Documentation: No API Documentation available for {api_name}

I see myself some months from now confusing the None <None> with a doc generation error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OriolAbril I understand your point. I have done the changes. Now, it'll show something like:

API Documentation: No API Documentation available

We can't specify the name of documentation available because api_name would be an open string and other params like modulename will be in the form of backend_xyz. If we must do this then we have to use regex again or split the name and then pass to the new variable.


.. literalinclude:: {fname}
:lines: {end_line}-
Expand All @@ -54,6 +55,7 @@ def execfile(filename, globals=None, locals=None):
:source-position: none

**Python source code:** :download:`[download source: {fname}]<{fname}>`
**API documentation:** `{api_name} <../../generated/arviz.{api_name}>`_

.. literalinclude:: {fname}
:lines: {end_line}-
Expand Down Expand Up @@ -236,6 +238,16 @@ def thumbfilename(self):
pngfile = self.modulename + "_thumb.png"
return pngfile

@property
def apiname(self):
name=""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just return None if nothing is found?

Copy link
Contributor Author

@percygautam percygautam Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, make sense.

with open(op.join(self.target_dir, self.pyfilename), "r") as file:
regex = r"az\.(plot\_[a-z_]+)\("
percygautam marked this conversation as resolved.
Show resolved Hide resolved
matches = re.finditer(regex, file.read(), re.MULTILINE)
for matchNum, match in enumerate(matches, start=1):
name = match.group(1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this return the last found plot_xyz? Should it return all the matches?

Do you think re.findall would work here? No need for looping?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the first is enough, there is only a couple of cases where more than a function is called, and in these cases the function called is always the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahartikainen @OriolAbril I worked some examples and let's say we have a hypothetical code as:

az.plot_compare(model_compare, figsize=(12, 4))
az.plot_forest(model_compare, figsize=(12, 4))
az.plot_density(model_compare, figsize=(12, 4))

If we run our code, it will return the last found match as 'plot_density'. As in our example files, we are only calling the function one time, so I don't think it'll be a problem.
As a possible solution for, when (in future) we have multiple plots in a single example, we can do something like:

    for matchNum, match in enumerate(matches, start=1):
        api_name.append(match.group(1))
    return api_name #returns the list

And then maybe check the length when rendering it in the template.
As suggested by @ahartikainen , re.findall() will work best for such case. For example, above code can be simplified as

with open("mpl_plot_compare.py", "r") as f:
    regex = r"az\.(plot\_[a-z_]+)\("
    api_name = re.findall(regex, f.read())
return api_name  #api_name will contain list

How should we do it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using only the first function is good enough for that. Returning the list with all occurrences seems like an overkill which complicates the code without much gain

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll do it.

return name

@property
def sphinxtag(self):
return self.modulename
Expand Down Expand Up @@ -380,6 +392,7 @@ def main(app):
fname=ex.pyfilename,
absfname=op.join(target_dir, ex.pyfilename),
img_file=ex.pngfilename,
api_name=ex.apiname,
)
with open(op.join(target_dir, ex.rstfilename), "w") as f:
f.write(output)
Expand Down