Skip to content

Fix #25032 by reading gallery ordering from a txt file in galleries folder #27991

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

NirobNabil
Copy link
Contributor

@NirobNabil NirobNabil commented Mar 29, 2024

PR summary

Addresses #25032. Requires review for closing #25032

Modified gallery_order.py to read ordering of examples from galleries/plot_types and its subdirectories. It adresses the issue described in #25032 but instead of reading the ordering from readme it's currently fetching ordering from gallery_order.txt as described in the comments by @timhoffm and @story645.

I only added the functionality for galleries/plot_types as a draft to gather feedback on the approach. After iterations if the implementation approach is finalized I can work on integrating with other galleries and also add the devel documentation as asked in #25032

PR checklist

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@NirobNabil NirobNabil marked this pull request as ready for review March 29, 2024 13:25
@github-actions github-actions bot added the Documentation: plot types files in galleries/plot_types label Mar 29, 2024
@story645 story645 added Documentation: build building the docs and removed Documentation: plot types files in galleries/plot_types labels Mar 29, 2024
Comment on lines 66 to 73
# plot_types_order = [
# '../galleries/plot_types/basic',
# '../galleries/plot_types/stats',
# '../galleries/plot_types/arrays',
# '../galleries/plot_types/unstructured',
# '../galleries/plot_types/3D',
# UNSORTED
# ]
Copy link
Member

Choose a reason for hiding this comment

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

if this is no longer needed, it can be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed them in latest commit

Comment on lines +76 to +77
plot_types_directory = "../galleries/plot_types/"
plot_types_order = get_ordering(plot_types_directory, include_directory_path=True)
Copy link
Member

Choose a reason for hiding this comment

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

does this solution generalize? (to the other folders?) I like that it looks relatively lightweight

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it does, because other folders are also structured in the same way as plot_types directory. Except for the users_explain directory, because its not included in explicit_order_folders but some of it's files are in list_all. I'm not sure why users_explain directory is not included in explicit_order_folders but because of this, i think its currently being sorted according to default values but that doesn't seem to totally line up with my understanding. because the sort keys returned by MplExplicitOrder will be UNSORTED thus defaulting to alphabetic ordering or LOC based ordering. But that doesn't seem to be the case when i'm going through the user guide section in doc website. Any clarification on how user_explain is sorted will help me greatly.

Other than that, rest of the folders will work fine with my current implementation as far as i understand. I'll push an update soon with the modifications for other other folders.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for not explaining this earlier, user_explain doesn't have a gallery landing page, instead it uses https://github.com/matplotlib/matplotlib/blob/main/doc/users/index.rst

@github-actions github-actions bot added Documentation: plot types files in galleries/plot_types Documentation: examples files in galleries/examples and removed Documentation: build building the docs labels Mar 29, 2024
@NirobNabil
Copy link
Contributor Author

NirobNabil commented Mar 29, 2024

Completely excluded user guide and tutorials page from gallery_order.py as their ordering is hardcoded in /doc/users/index.rst and galleries/tutorials/index.rst respectively. Asking for feedback on this approach.

Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

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

looking great, sorry for dropping the ball on this. for the flake errors, have you installed the pre-commit hooks? https://matplotlib.org/devdocs/devel/development_setup.html#install-pre-commit-hooks

They should autofix that error.

Comment on lines +13 to +25
file_path = os.path.join(dir, 'gallery_order.txt')
f = open(file_path, "r")
lines = [line.replace('\n', '') for line in f.readlines()]
ordered_list = []
for line in lines:
if line == "unsorted":
ordered_list.append(UNSORTED)
else:
if include_directory_path:
ordered_list.append(os.path.join(dir, line))
else:
ordered_list.append(line)

Copy link
Member

@story645 story645 May 9, 2024

Choose a reason for hiding this comment

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

Suggested change
file_path = os.path.join(dir, 'gallery_order.txt')
f = open(file_path, "r")
lines = [line.replace('\n', '') for line in f.readlines()]
ordered_list = []
for line in lines:
if line == "unsorted":
ordered_list.append(UNSORTED)
else:
if include_directory_path:
ordered_list.append(os.path.join(dir, line))
else:
ordered_list.append(line)
ordered_list = []
with open(os.path.join(dir, 'gallery_order.txt'), 'r') as f:
for line in f.readlines():
line = line.rstrip("\n")
if line == "unsorted":
ordered_list.append(UNSORTED)
elif include_directory_path:
ordered_list.append(os.path.join(dir, line))
else:
ordered_list.append(line)

um this is mostly stylistic, but it does remove one loop and I think you should be able to use context managers here

def list_directory(parent_dir):
"""Return list of sub directories at a directory"""
root, dirs, files = next(os.walk(parent_dir))
return [os.path.join(root, dir) for dir in dirs]

# Gallery sections shall be displayed in the following order.
# Non-matching sections are inserted at the unsorted position

UNSORTED = "unsorted"
Copy link
Member

Choose a reason for hiding this comment

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

should maybe be moved to top of file if it's being used in the listing function above (or passed in as a variable)

Comment on lines +76 to +77
plot_types_directory = "../galleries/plot_types/"
plot_types_order = get_ordering(plot_types_directory, include_directory_path=True)
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for not explaining this earlier, user_explain doesn't have a gallery landing page, instead it uses https://github.com/matplotlib/matplotlib/blob/main/doc/users/index.rst

Comment on lines +78 to +80
except FileNotFoundError:
# Fallback to ordering already defined in list_all
pass
Copy link
Member

Choose a reason for hiding this comment

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

can this be logged to sphinx just in case this ignores something that shouldn't be ignored?

@story645
Copy link
Member

@NirobNabil any bandwidth for getting back to this or can we label this as orphaned?

@timhoffm
Copy link
Member

timhoffm commented Sep 26, 2024

Was there a design discussion on this? Note that since sphinx-gallery 0.16.0, subsection order can be a simple list with optionally one wildcard '*' (https://sphinx-gallery.github.io/stable/configuration.html#sorting-gallery-subsections).

I believe the current ordering can be achieved without any code by:

  1. making the sectionorder1 an explicit list of all sections. There are not that many that we must add UNSORTED logic.
  2. making the subsectionorder1 a list of the currently sorted examples and a '*' wildcard at the end.
Example: (click to expand)
# Gallery subsections shall be displayed in the following order.
# Note that sphinx-gallery only has one large subsection_order config
# for galleries, one can put the orders of different galleries simply
# beneath each other, because all that matters is the relative position
# of the subsections with respect to other subsections of the same
# gallery.
subsection_order = [
    # examples
    '../galleries/examples/lines_bars_and_markers',
    '../galleries/examples/images_contours_and_fields',
    '../galleries/examples/subplots_axes_and_figures',
    '../galleries/examples/statistics',
    '../galleries/examples/pie_and_polar_charts',
    '../galleries/examples/text_labels_and_annotations',
    '../galleries/examples/color',
    '../galleries/examples/shapes_and_collections',
    '../galleries/examples/style_sheets',
    '../galleries/examples/pyplots',
    '../galleries/examples/axes_grid1',
    '../galleries/examples/axisartist',
    '../galleries/examples/showcase',
    '../galleries/examples/animation',
    '../galleries/examples/event_handling',
    '../galleries/examples/misc',
    '../galleries/examples/mplot3d',
    '../galleries/examples/scales',
    '../galleries/examples/speciality_plots',
    '../galleries/examples/spines',
    '../galleries/examples/ticks',
    '../galleries/examples/units',
    '../galleries/examples/user_interfaces',
    '../galleries/examples/widgets',
    '../galleries/examples/userdemo',
    # tutorials
    #   currently doesn't have subfolders
    # plot_types
    '../galleries/plot_types/basic',
    '../galleries/plot_types/stats',
    '../galleries/plot_types/arrays',
    '../galleries/plot_types/unstructured',
    '../galleries/plot_types/3D',
]

within_subsection_order = [
    #  **Tutorials**
    #  introductory
    "quick_start", "pyplot", "images", "lifecycle", "customizing",
    #  intermediate
    "artists", "legend_guide", "color_cycle",
    "constrainedlayout_guide", "tight_layout_guide",
    #  advanced
    #  text
    "text_intro", "text_props",
    #  colors
    "colors",

    #  **Examples**
    #  color
    "color_demo",
    #  pies
    "pie_features", "pie_demo2",

    # **Plot Types
    # Basic
    "plot", "scatter_plot", "bar", "stem", "step", "fill_between",
    # Arrays
    "imshow", "pcolormesh", "contour", "contourf",
    "barbs", "quiver", "streamplot",
    # Stats
    "hist_plot", "boxplot_plot", "errorbar_plot", "violin",
    "eventplot", "hist2d", "hexbin", "pie",
    # Unstructured
    "tricontour", "tricontourf", "tripcolor", "triplot",
    # Spines
    "spines", "spine_placement_demo", "spines_dropped",
    "multiple_yaxis_with_spines", "centered_spines_with_arrows",
    "*",  # all non-listed examples are appended to the end of their respective section
]

This simplifies the structure as it's purely declarative. We could even rid of our gallery_order.py file and inline this into conf.py.

This is simple and may be good enough. The only downside is that sphinx-gallery does not have per-gallery order. All order information must be placed into the two lists subsection_order and within_subsection_order.


If we really want individual config files (and are willing to write and maintain the code logic for that). I suggest to go with one structured file per gallery (e.g. YAML) so that a gallery_order.yml could look like

- basic
  - plot
  - scatter_plot
  - bar
  - stem
  - step
  - fill_between
- arrays
  - imshow
  - pcolormesh
  - contour
  - contourf
[...]

Footnotes

  1. Naming is sloppy: we call this sectionorder and subsectionorder; sphinx-gallery calls this subsection_order and within_subsection_order. 2

@NirobNabil
Copy link
Contributor Author

@story645 I'm getting back to this tonight. sorry totally forgot about it in the inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation: examples files in galleries/examples Documentation: plot types files in galleries/plot_types status: needs rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MNT]: Specify ordering in file in gallery folder
3 participants