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

feature matrix - fix path handling for windows and linux support #9063

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

cderv
Copy link
Collaborator

@cderv cderv commented Mar 13, 2024

Currently it does not work for me if I try to render the feature matrix on windows.

I suggest we use pathlib to handle file path instead of using split() and some separator which would require to handle each case.

pathlib is from python 3.4 BTW https://docs.python.org/3/library/pathlib.html

I am opening this PR so that you can check this is working for you.

@gordonwoodhull I can make a PR or a commit to your branch. Otherwise here is a patch

diff --git a/dev-docs/feature-format-matrix/create_table.py b/dev-docs/feature-format-matrix/create_table.py
index 4989aecca..28bd4f3b5 100644
--- a/dev-docs/feature-format-matrix/create_table.py
+++ b/dev-docs/feature-format-matrix/create_table.py
@@ -1,6 +1,6 @@
 import yaml
 import json
-import glob
+import pathlib
 
 class Trie:
 
@@ -104,11 +104,12 @@ def table_cell(entry, _feature, _format_name, format_config):
 def compute_trie(detailed = False):
     trie = Trie()
     pattern = "qmd-files/**/*.qmd" if detailed else "qmd-files/**/document.qmd"
-    for entry in glob.glob(pattern, recursive=True):
+    for entry in pathlib.Path(".").glob(pattern):
+        pathParts = entry.parts
         if detailed:
-            feature = entry.split("/")[1:]
+            feature = pathParts[1:]
         else:
-            feature = entry.split("/")[1:-1]
+            feature = pathParts[1:-1]
         front_matter = extract_metadata_from_file(entry)
         try:
             format = front_matter["format"]

@cderv cderv requested a review from cscheid March 13, 2024 17:38
@cscheid
Copy link
Collaborator

cscheid commented Mar 13, 2024

That "/".split() is on me, as usual 🤦

Copy link
Collaborator

@cscheid cscheid left a comment

Choose a reason for hiding this comment

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

If this works on unix + windows, then 👍

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Mar 13, 2024

Those pesky slashes and backslashes! Please go ahead and merge this and I will rebase #9052.

It runs on macos.

@cscheid cscheid merged commit 566aeca into main Mar 13, 2024
47 checks passed
@cscheid cscheid deleted the feature-matrix/windows-support branch March 13, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants