Incremental builder #678
Incremental builder #678
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
|
I'm afraid I'm at a loss with the remaining type linting error It seems like |
Arelle is not considered a typed library, so imports from Arelle are treated as Any, and subclassing Any is disallowed. You could use class IXBRLViewerPluginData(PluginData): # type: ignore[misc]to ignore it once, or you could add |
Ah, thank you. I'd assumed that as we reference Arelle types elsewhere that we were making use of Arelle type information, e.g.: def addFact(self, report: ModelXbrl, f):But I assume that sub-classing is a special case where the type information is actually needed? OOI, what is needed for Arelle to be considered a typed library? Is it just missing type annotations, or is there some package configuration needed? |
|
ModelXbrl is also typed as Any there, but Any is compatible with every type, so type errors are rare. One exception is subclassing, which is blocked by disallow_subclassing_any, enabled by mypy strict mode. We'd need to choose one of the options to support typing. Alternatively, mypy might implement a feature to consider a module as providing types, even if it's not officially supported. |
|
@paulwarren-wk The |
Thank you - that's much clearer now. I've now added the comment to ignore type checks on this line (d5cccaa) |
iXBRLViewerPlugin/__init__.py
Outdated
| @@ -300,12 +319,13 @@ def guiRun(cntlr, modelXbrl, attach, *args, **kwargs): | |||
| for c in FEATURE_CONFIGS | |||
| if cntlr.config.setdefault(f'{CONFIG_FEATURE_PREFIX}{c.key}', False) | |||
| ] | |||
| pluginData(cntlr).builder = IXBRLViewerBuilder(cntlr, useStubViewer = True) | |||
| processModel(modelXbrl) | |||
There was a problem hiding this comment.
This is missing the cntlr parameter.
When run from the GUI:
[viewer:exception] Exception processModel() missing 1 required positional argument: 'modelXbrl'
Traceback [' File "/Users/austinmatherne/git/Arelle/arelle/plugin/iXBRLViewerPlugin/__init__.py", line 323, in guiRun\n processModel(modelXbrl)\n'] - wk-20230930.htm There was a problem hiding this comment.
Fixed in 36f1c09
|
@paulwarren-wk @austinmatherne-wk Hi Guys, I hope you had an amazing weekend! :) This is in regards to the comment on #653 (reference). Does this pull request resolve the bug? It appears that there were conflicts with the pull? Thank you for your valuable time, Chad |
Sorry @michaelchadfleming, I dropped the ball on this and didn't realise it was blocked on me. I've just pushed a fix for what I think is the final issue. |
Reason for change
Fixes #653. Avoids retaining loaded XBRL models indefinitely.
Description of change
Previously the viewer used the
keepOpenfeature in Arelle to retain XBRL models after the are loaded, so that multiple files can be assembled into a single viewer. This change uses the newpluginDatafeature to preserve viewer information across models when building multi-document viewers, and then creating the viewer in a separate call after all models are processed.A slightly awkward feature of this is that we need to retain the XML models until we serialize at the end, but the XML models created by Arelle use custom element classes that are no longer safe to use once the Arelle models have been disposed. I have fixed this by unsetting the custom element class lookup after taking a copy of the model.
Steps to Test
See steps on #653. Can also be confirmed by submitting the same file to the web service multiple times, and confirming that you get exactly the same file back each time.
review:
@Arelle/arelle
@paulwarren-wk