plugin is outdated

[terokorp]

Plugin banking.statements.osuuspankki is outdated and dosen't work, but it has one fork which works fine. https://github.com/vukk/banking.statements.osuuspankki
Would it be better to put to plugins list instead of that broken one?

[petri]

Issue reports and pull requests welcome. (I am the author of b.s.o.)

[terokorp]

I'm just user who tried to use b.s.o and got weird errors. I pinpointed problem to transaction type maping and then find out that someone has already fixed those problems.
( vukk/banking.statements.osuuspankki@76b942e )
So, link to working version is on my first comment and that is not my repo and I don't want to send pull reuqests from others repos, if that's even possible.

Original b.s.o repo was been so long time untouced (abt 2½ year) and fix for this bug has been made over one year ago, so I thought original b.s.o was already dead.

[petri]

Thats's all fine and dandy @terokorp, I understand. Can you submit a bug report to b.s.o? And @vukk, could you please submit a pull request after that?

[vukk]

Since there is no issue raised at b.s.o yet, I'll note this here:
My fork removes functionality, namely the transaction types. Some (many?) consumers of ofx statements only support CREDIT/DEBIT types, so it's a good default. I think this is what is observed here.

But maybe the additional types should be made optional (e.g. config file) instead of fully removing them.

Also the fork code is untested on OP corporate CSVs, since I did not have access to these for testing.

[petri]

Thanks for explaining, @vukk. AFAIK, ofxstatement is not a tool for ONLY GnuCash, and multiple transaction types (there are almost twenty) are part of the OFX standard. The types are very useful and outright absolutely needed for some other use cases. Therefore, removing functionality is NOT an option in my opinion and this issue here reported by @terokorp should be closed as invalid.

However, I do agree that some sort of "compatibility mode" would be a good idea and it's good that this issue of compatibility was brought up. Perhaps the matter does merit its own (new) issue then.

BUT - the implementation would have to be supported by all the plugins and driven by the author of ofxstatement... @kedder, what's your thoughts on this?

[petri]

I went and added #28 for this. Please let's continue the discussion there? (and close this issue)

[kedder]

Hm, guys, do you think it would be reasonable to make the output configurable on b.s.o level, in similar way this plugin is configured: https://github.com/kedder/ofxstatement-lithuanian/blob/master/src/ofxstatement/plugins/danske.py#L47. That way user can ask the plugin to produce the output he wants (with the sensible default, of course).

Would it make both kinds of users happy (ones that use it for GnuCash, and ones that need more precise ofx markup)?

[petri]

Configuring the plugin using a settings attribute defined in the base class seems fine to me, if that's what you mean.

But please see my suggestion at #28 ... Being able to set a value to settings does not mean the plugin actually implements anything. It should be possible for an user to (programmatically) find out if a functionality (configured using the settings) is supported by the plugin.

So please, let's also add something to the base class that plugins can use to advertise the features they support.

As far as I am concerned, this could be as simple as having the base class implement a boolean attribute for each feature originally set to False, which each plugin could then override if it implements the feature.

The base class in ofxstament:

class Plugin(object):
    ui = None

    FEATURE_GNUCASH_OFX = False

    def __init__(self, ui, settings):
         self.ui = ui
         self.settings = settings

    def get_parser(self, filename):
         raise NotImplementedError()

And the custom plugin:

class MyGnuCashSupportingPlugin(Plugin):
    FEATURE_GNUCASH_OFX = True
    (... rest of implementation here ...)

The above mechanism means that all features have to be managed via ofxstatement. Some would say it's a drawback. But I don't think it is a drawback but rather a good way of making sure everyone uses the same values for the features - which is a MUST.

If we instead define a capabilities API similar as to how settings are managed in the base class in ofxstament, we allow plugin authors to choose the capability names and values as they wish:

class Plugin(object):
    ui = None

    def __init__(self, ui, settings):
         self.ui = ui
         self.settings = settings
         self.capabilities = {}

    def get_parser(self, filename):
         raise NotImplementedError()

Now, what if one author decides to say GnuCash OFX support is defined as self.capabilities["GNUCASH_OFX"] and another as self.capabilities["GNUCASH_RESTRICTED_OFX"] ???

Users will then not have a single way to determine if GnuCash OFX is supported by the plugins.

So whatever mechanism is used, please, @kedder, DO act as a benevolent dictator here, to define the capabilities :)

(BTW using a boolean attribute per feature also documents the features nicely)

[kedder]

(I'm not sure whether #28 is better place for this, but since discussion happens here, responding here as well)

@petri, I see some practical problems with this approach (ofxstatement core manages a list of all possible plugin "capabilities").

1. A capability, as you described it, is not just a boolean, it is an interface, that plugin promises to comply with. So ofxstatement suddenly have to document these interfaces, and, perhaps, provide a way to check for compliance. Otherwise, what does it mean for plugin to implement GNUCASH_RESTRICTED_OFX capability? Plugins will eventually implement a "feature" in different incompatible way and this central mechanism for reporting "capabilities" will became pretty useless.
2. There is a huge impedance for plugin authors to add a new "capability": they have to think how to make it generic enough so it can be usable/implementable by other plugins, they have to get their pull request accepted by ofxstatement maintainer. And after all (my own assessment), most of plugin authors couldn't care less about this stuff, they just want their statements parsed, so they won't be interested in compliance with these capabilities too much.
3. It will put a burdain on me to carefully understand all the features other plugins offer, maintain sensible and understandable list of capabilities, try to detect duplicate "capabilities". Also, there will be basically no practical way to refactor this list of capabilities, join some together into more "generic" ones, or split too generic capabilities into several more specific -- it will involve changing plugins that implement these capabilities, something I don't want to deal with.
4. Finally, this can only support boolean "flags", and doesn't address configuration for capabilities in any way.

This is why I would like to leave such features and variances in behaviour to plugins themselves, via their private configuration options. We can invest some effort into making supported configuration options more discoverable by users, but I'd like to have it in informal way (e.g. as docstring), rather than formal (e.g. configuration schemas, interfaces, etc), because complexity of formal approach seems too high for the purpose.

[petri]

I already suggested (see above) to continue this on #28 ... but:

My suggestion was meant to document the capabilities of plugins for the benefit of users of ofxstatement.

To rephrase, I did not mean to treat this as a matter of enforcing strict interface compliance - instead, more like a matter of documentation, really.

And perhaps I chose my words badly above when I spoke of "defining capabilities": I would merely hope that ofxstatement / @kedder could perhaps step in to do some arbitration if plugin authors implementing capabilities are doing it in a manner that is causing problems for users of plugins. Which should be a rare occasion.

But I am happy even without that - if we could just have some documented, recommended way of documenting the capabilities for the purposes of easily discovering and checking for them?

For that, a boolean flag in a capabilities dict is sufficient to me.

[MirkoDziadzka]

Hi

I'm not sure if the existing plugins are really the right place for these features.

As I understand them, they are more about converting an arbitrary input to OFX. Some of them are doing filtering. For example, I contributed stuff to merge some lines on some credit card statements. This was probably not the right place to do this but the only place I could added this back then.

Maybe a better place would be an additional OFX filter phase, where filter plugins can be registered to work on the OFX data and rewrite them before the output. This would make these features orthogonal to the input parsing and it would be possible to add an 'gnucash' filter which rewrites all the stuff that gnucash don't like.

[petri]

Please, @kedder can you close this so we can continue on #28? I changed its title to better reflect the issue at hand, and summarized what's been discussed here.

Whereas the title of this issue does not describe the discussion that we got into. Furthermore, this issue is also invalid (there is no bug in the plugin).

[kedder]

Ok