Added HDF5 file support Through Plugins#1884
Open
aaravmaloo wants to merge 8 commits intoQL-Win:masterfrom
Open
Added HDF5 file support Through Plugins#1884aaravmaloo wants to merge 8 commits intoQL-Win:masterfrom
aaravmaloo wants to merge 8 commits intoQL-Win:masterfrom
Conversation
Reviewer's GuideAdds a new built-in HDF5 viewer plugin that detects HDF5 files via signature probing and renders a metadata-focused, text-based tree summary of groups, datasets, and attributes within QuickLook, wired into the existing solution and build. Sequence diagram for HDF5 file preview handlingsequenceDiagram
actor User
participant QuickLookHost
participant Plugin
participant Hdf5SummaryBuilder
participant HDF5NET as HDF5_NET
User->>QuickLookHost: Open file path
QuickLookHost->>Plugin: CanHandle(path)
Plugin->>Plugin: Check extension against SupportedExtensions
Plugin->>Plugin: HasHdf5Signature(path)
Plugin-->>QuickLookHost: bool canHandle
alt canHandle is true
QuickLookHost->>Plugin: Prepare(path, context)
Plugin->>context: Set PreferredSize
QuickLookHost->>Plugin: View(path, context)
Plugin->>Plugin: Create Hdf5TextPanel
Plugin->>context: Set ViewerContent, Title, IsBusy = true
Plugin->>Hdf5SummaryBuilder: Build(path)
Hdf5SummaryBuilder->>HDF5NET: H5File.OpenRead(path)
HDF5NET-->>Hdf5SummaryBuilder: H5File
Hdf5SummaryBuilder->>HDF5NET: Traverse groups, datasets, attributes
HDF5NET-->>Hdf5SummaryBuilder: Metadata objects
Hdf5SummaryBuilder-->>Plugin: string summaryText
Plugin->>Hdf5TextPanel: SetText(summaryText)
Plugin->>context: IsBusy = false
else error while building summary
Plugin->>Hdf5TextPanel: SetText(errorMessage)
Plugin->>context: IsBusy = false
end
QuickLookHost-->>User: Display HDF5 metadata tree summary
Class diagram for new HDF5 viewer plugin typesclassDiagram
class Plugin {
+int Priority
+void Init()
+bool CanHandle(string path)
+void Prepare(string path, ContextObject context)
+void View(string path, ContextObject context)
+void Cleanup()
-Hdf5TextPanel _panel
-static byte[] Hdf5Signature
-static string[] SupportedExtensions
-static bool HasHdf5Signature(string path)
}
class Hdf5TextPanel {
+Hdf5TextPanel()
+void SetText(string text)
-TextBox _textBox
}
class Hdf5SummaryBuilder {
+static string Build(string path)
-static void AppendObject(H5Object h5Object, StringBuilder sb, int depth)
-static void AppendGroup(H5Group group, StringBuilder sb, int depth)
-static void AppendDataset(H5Dataset dataset, StringBuilder sb, int depth)
-static void AppendAttributes(H5AttributableObject attributable, StringBuilder sb, int depth)
-static IEnumerable~H5Object~ SafeReadChildren(H5Group group)
-static string SafeName(string name)
-static string Indent(int level)
-const int MaxDepth
-const int MaxChildrenPerGroup
-const int MaxAttributesPerObject
}
class IViewer {
<<interface>>
+int Priority
+void Init()
+bool CanHandle(string path)
+void Prepare(string path, ContextObject context)
+void View(string path, ContextObject context)
+void Cleanup()
}
class ContextObject {
+Size PreferredSize
+object ViewerContent
+string Title
+bool IsBusy
}
class H5Object
class H5Group
class H5Dataset
class H5CommitedDatatype
class H5UnresolvedLink
class H5AttributableObject
class H5Attribute
class UserControl
class TextBox
Plugin ..|> IViewer
Hdf5TextPanel ..|> UserControl
Hdf5SummaryBuilder ..> H5Object
Hdf5SummaryBuilder ..> H5Group
Hdf5SummaryBuilder ..> H5Dataset
Hdf5SummaryBuilder ..> H5CommitedDatatype
Hdf5SummaryBuilder ..> H5UnresolvedLink
Hdf5SummaryBuilder ..> H5AttributableObject
Hdf5SummaryBuilder ..> H5Attribute
Plugin ..> Hdf5TextPanel
Plugin ..> Hdf5SummaryBuilder
Plugin ..> ContextObject
Hdf5TextPanel ..> TextBox
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
Plugin.Cleanupyou callGC.SuppressFinalize(this)even thoughPluginhas no finalizer; consider removing this call to avoid implying a finalization pattern that isn’t actually used. - In
HasHdf5Signature, theprobesarray is allocated and initialized on every call; you could move this to a static readonly field or generate the offsets lazily to reduce per-call allocations in scenarios whereCanHandleis invoked frequently. - In
View, you always setcontext.IsBusy = falseinfinallybut never set it totrue; if the host expects the viewer to manage this flag, consider explicitly setting it totruebefore starting work for clearer state transitions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Plugin.Cleanup` you call `GC.SuppressFinalize(this)` even though `Plugin` has no finalizer; consider removing this call to avoid implying a finalization pattern that isn’t actually used.
- In `HasHdf5Signature`, the `probes` array is allocated and initialized on every call; you could move this to a static readonly field or generate the offsets lazily to reduce per-call allocations in scenarios where `CanHandle` is invoked frequently.
- In `View`, you always set `context.IsBusy = false` in `finally` but never set it to `true`; if the host expects the viewer to manage this flag, consider explicitly setting it to `true` before starting work for clearer state transitions.
## Individual Comments
### Comment 1
<location path="QuickLook.Plugin/QuickLook.Plugin.Hdf5Viewer/Hdf5SummaryBuilder.cs" line_range="85-94" />
<code_context>
+ AppendAttributes(dataset, sb, depth + 1);
+ }
+
+ private static void AppendAttributes(H5AttributableObject attributable, StringBuilder sb, int depth)
+ {
+ List<H5Attribute> attributes;
+
+ try
+ {
+ attributes = attributable.Attributes.ToList();
+ }
+ catch (Exception ex)
+ {
+ sb.AppendLine($"{Indent(depth)}@attributes: <error: {ex.Message}>");
+ return;
+ }
+
+ if (!attributes.Any())
+ return;
+
+ var visible = attributes.Take(MaxAttributesPerObject).ToList();
+
+ foreach (var attribute in visible)
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid materializing the full attribute collection when only a capped subset is needed.
`attributes = attributable.Attributes.ToList();` forces enumeration and allocation of the full attribute set even though we only use up to `MaxAttributesPerObject`. For large files this can be costly. Consider iterating lazily and stopping after `MaxAttributesPerObject + 1` items (to determine whether to show the "more attributes" line) instead of materializing the entire list.
Suggested implementation:
```csharp
private static void AppendAttributes(H5AttributableObject attributable, StringBuilder sb, int depth)
{
var visible = new List<H5Attribute>(MaxAttributesPerObject);
var hasMoreAttributes = false;
try
{
var count = 0;
foreach (var attribute in attributable.Attributes)
{
count++;
if (count <= MaxAttributesPerObject)
{
visible.Add(attribute);
}
else
{
hasMoreAttributes = true;
break;
}
}
}
catch (Exception ex)
{
sb.AppendLine($"{Indent(depth)}@attributes: <error: {ex.Message}>");
return;
}
if (visible.Count == 0)
return;
foreach (var attribute in visible)
```
```csharp
if (hasMoreAttributes)
```
</issue_to_address>
### Comment 2
<location path="QuickLook.Plugin/QuickLook.Plugin.Hdf5Viewer/Plugin.cs" line_range="38-47" />
<code_context>
+ context.PreferredSize = new Size { Width = 1100, Height = 760 };
+ }
+
+ public void View(string path, ContextObject context)
+ {
+ _panel = new Hdf5TextPanel();
+ context.ViewerContent = _panel;
+ context.Title = Path.GetFileName(path);
+
+ try
+ {
+ _panel.SetText(Hdf5SummaryBuilder.Build(path));
+ }
+ catch (Exception ex)
+ {
+ _panel.SetText(
+ $"Failed to open HDF5 file.{Environment.NewLine}{Environment.NewLine}" +
+ $"{ex.GetType().Name}: {ex.Message}");
+ }
+ finally
+ {
+ context.IsBusy = false;
+ }
+ }
</code_context>
<issue_to_address>
**suggestion (performance):** Synchronous HDF5 parsing in View may block the UI thread for large files.
`Hdf5SummaryBuilder.Build(path)` is executed directly in `View`, so any heavy I/O or parsing work will run on the UI thread. If the plugin API supports it, run the summary build on a background task and update `_panel` when it finishes, keeping `context.IsBusy` true until then to avoid UI hangs for large/remote files.
Suggested implementation:
```csharp
public void Prepare(string path, ContextObject context)
{
context.PreferredSize = new Size { Width = 1100, Height = 760 };
}
public void View(string path, ContextObject context)
{
_panel = new Hdf5TextPanel();
context.ViewerContent = _panel;
context.Title = Path.GetFileName(path);
Task.Run(() =>
{
try
{
return Hdf5SummaryBuilder.Build(path);
}
catch (Exception ex)
{
return
$"Failed to open HDF5 file.{Environment.NewLine}{Environment.NewLine}" +
$"{ex.GetType().Name}: {ex.Message}";
}
}).ContinueWith(t =>
{
_panel.SetText(t.Result);
context.IsBusy = false;
}, TaskScheduler.FromCurrentSynchronizationContext());
}
public bool CanHandle(string path)
```
1. Ensure the file has `using System.Threading.Tasks;` at the top:
- If there is a using block, add:
`using System.Threading.Tasks;`
2. This implementation assumes `View` is called on the UI thread and that a synchronization context is present so `TaskScheduler.FromCurrentSynchronizationContext()` marshals back correctly. If the plugin framework exposes its own dispatcher or marshalling API on `ContextObject` (e.g. `context.Post(...)`), replace `TaskScheduler.FromCurrentSynchronizationContext()` with the framework-specific mechanism.
3. If `context.IsBusy` is not already set to `true` before `View` is called, you may also want to set `context.IsBusy = true;` at the start of `View` to indicate work is in progress.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
the previous and this commit basically fixed the issues reported by sourceery-ai on the pr
Member
How about the Suggested Alternatives PureHDF? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Functionality has been tested, no obvious bugs
Code style follows project conventions
Documentation/comments updated (if applicable)
Brief Description of Changes
Please briefly describe the main changes in this PR:
Added a new built-in plugin project: QuickLook.Plugin.Hdf5Viewer
Implemented HDF5 file handling for .h5, .hdf5, .hdf, and .he5
Added robust HDF5 signature detection (including user-block offset probing) before claiming files
Implemented a metadata-first preview that renders:
group hierarchy
dataset metadata (shape, type class, item size, layout)
attribute metadata (type, shape, item size)
Added a simple WPF text viewer panel (Hdf5TextPanel) for readable tree output
Added traversal/size limits in summary builder to prevent UI issues on very large files
Wired the project into solution and build integration:
QuickLook.sln
QuickLook.slnx (including installer build dependency)
Related Issue (if any)
Please provide related issue numbers:
Solves #1878
Additional Notes
Add any extra notes here:
Uses managed NuGet dependency HDF5.NET (1.0.0-alpha.22) to avoid native runtime dependencies.
Summary by Sourcery
Add a built-in HDF5 viewer plugin that previews HDF5 file structure and metadata within QuickLook.
New Features:
Build: