[WIP] Organizational review

[Funth0mas]

Organizational review summary:

• Add canmatrix.formats package
• Unify/define formats interface, consider to use zope.interface or simply abc.abstractclass
• canmatrix.canmatrix module should be renamed - canmatrix.core ??
• Possibly expose a particular few things via canmatrix.some_thing() - what classes ??
• Decide and apply a consistent import 'format'
  • No from module import * (!)
• Isolate CLI from library rather than colocating (use cli or cmd module?)
• Use CLICK instead of optparse
• pathlib2 or os.path ?
  • Well, decide one way or the other, altendky preference is for pathlib stuff.
• snake_case for all except of classes, classes with UpperCamel - in progress
• loadb for loading any format from a byte sequence
• static typing
• mypy running in CI
• check and resolve class-wise duplicities (del_frame() vs remove_frame(), comment vs comments)
• remove useless cm prefix from some files
• ??

[altendky]

I'm not big on the cm prefixes as we already have the canmatrix package (the outermost one). It would be nice to never canmatrix.canmatrix as the second one really describes nothing useful.

As to interfaces, my impression from discussions in #python is that zope.interface is the thing to learn from and use, not stdlib (such as abc).

Anyways, you are talking about moving canmatrix.dbc to canmatrix.converters.dbc? If so they are presently referred to as formats I think?

[altendky]

If we're talking about an overall organizational review (I'd rather make an overall plan first then shift everything around, I'm sure some opinions will change as we discuss various points) then I'll add that we might need a misc/utils or whatever module for things like grouper() (unless we use one of the libraries that provides tools like this).

https://github.com/ebroecker/canmatrix/pull/224/files#diff-c1624d42e3750ac47cb2bbd3c178a5d0R536

[Funth0mas]

Bad name, of course I thought formats.
Let cancel this ticket and make it somehow better...later... But I'm afraid I won't help a lot with it :-(

[Funth0mas]

Concerning the prefix, only now I get the cm stands for 'canmatrix', not 'converter'. Yes, I'm not analphabet, I see the converter is not comverter :-) . Does not matter.

[altendky]

We can rename the ticket or make a new one, no matter to me. I see a few things at the moment, let's build the list if there are more. If we are expecting to get more tests in place soone than later, maybe we continue that first before any reorganization.

• Add canmatrix.formats package
• canmatrix.canmatrix module should be renamed
• Possibly expose a particular few things via canmatrix.some_thing()
• Decide and apply a consistent import 'format'
  • No * imports
• Isolate CLI from library rather than colocating
• pathlib2 over os.path
  • Well, decide one way or the other, my preference is for pathlib stuff.
• lowerCamel or snake_case?

[ebroecker]

my 2 cents about overall organizational review

• I want to create a release (0.7) more or less the current development state (gonna check if some bugs should be fixed)
• after that I am open to abandon python2 support
• after that I am very open for cleaning up the code and reorganizing it
• certainly it would also be an option to work for these things in a branch.

@altendky , @Funth0mas : What would you prefer?

[altendky]

• This isn't urgent for me so waiting until after a release seems fine.
• I expected to give more notice than that for dropping py2. I don't know how we go about making sure people notice but I would say we at least have a release which notifies of imminent loss of py2 support before releasing a version that doesn't support py2 (or makes no promises of support for it). I only brought it up because I don't want to be continuing to support it in 2025 simply because we didn't get around to warning anyone it was going away.
• Definitely do it in a branch. Definitely don't let the branch linger. Yeah, git can handle moves but it's just another layer and let's not go begging for merge trouble.

How about we add another thing before a reorg. I don't recall offhand how inconsistent we are but let's pick an import style policy and implement it first. I personally nearly always use the form import canmatrix.formats then canmatrix.formats.load(). As opposed to from canmatrix.formats import load or import canmatrix.formats as f or from . import formats or... If we are reorganizing we'll be changing imports. If we are changing imports we might end up changing lots of names used in code. Mixing it all together seems not great. (I added this to the list of overhauls to consider)

[altendky]

added the following to the list

• Isolate CLI from library rather than colocating

[Funth0mas]

• don't use import *

[ebroecker]

• loadb for loading any format from a byte sequence (canmatrix.formats.loads() fails due to undefined StringIO #103)

[altendky]

@ebroecker, I saw this as building a list of 'policies'. Do we need an 0.8 ticket? Or milestone? (never worked with milestones in github) or tag?

[ebroecker]

I thought it's right here because the policy would be to use "byte sequences". If not - just ignore...

[Funth0mas]

@altendky the 'isolate CLI from library' means to move the two CLI scripts to another package, kind of canmatrix.cli, and update the `setup.py?

[Funth0mas]

I investigated the zope.interface and as far as I can see it has no real benefit unless we would like to use zope component system, what might be an overhead. Moreover the zope.interface support for mypy static typecheck is still in progress, actually leading to lot of mypy errors.
The zope interface is not meant to enforce an interface or even verify that this interface is implemented. I think the abc.abstractclass is pretty enough to define, that there is (can be) load and dump function in each format.
What I find more important is to refactor the formats to become classes, not only modules. What kind of interface definition to use is than minor question.

[altendky]

I know I'm way way late here...

I investigated the zope.interface and as far as I can see it has no real benefit unless we would like to use zope component system, what might be an overhead. Moreover the zope.interface support for mypy static typecheck is still in progress, actually leading to lot of mypy errors.

Certainly zope.interface is usable on it's own. It lets you describe an interface and express that something else satisfies that interface. All without being super infectious and annoying.

The zope interface is not meant to enforce an interface or even verify that this interface is implemented. I think the abc.abstractclass is pretty enough to define, that there is (can be) load and dump function in each format.

You can write tests which verify implementation of an interface. On the other hand yes you can use ABC to infect everything with a metaclass which basically keeps anyone else that uses this from using their own metaclasses. This exclusivity alone is sufficient to make me walk away from metaclasses. Though yes, ABC can be used without the metaclass at which point it's a lot more similar to zope.interface in that you do separate explicit checks that a given class implements an interface. Though I still don't end up seeing ABC recommended much.

Anyways, as much as I haven't been actively following canmatrix this year I have been watching enough to see all the work you are putting into it. Thanks for helping keep it improving, @Funth0mas .

[Funth0mas]

Thanks for the comment, maybe we need neither zope nor abc, just plain classes. Still better than functions on module level sharing data over global variables and fighting with python import system.

Next refactoring steps could be renaming canmatrix,py to core.py, finish codestyle cleanup on this file (snake_case etc.; thanks to mypy coverage it's quite secure job now) and than maybe first moving formats to classes. But such change will make the API completely incompatible.

[ebroecker]

@Funth0mas
I agree.
But I want to do a release just before making the API incompatible.

What do you think about releasing more or less "as is" as canmatrix 1.x
And start with new API and without py2 release this laster as canmatrix 2.x?

What do you think about:

• create a new release from current state as canmatrix 1.0
• afterwards we use development branch, kick out py2 and start with your proposed tasks

[altendky]

So a major version number that will only ever (be planned to) have a single release? Seems a bit funny, but maybe it doesn't matter. This seems like maybe just more pre-1.0 thrash. But, I really don't feel strongly about this. To a degree I don't really care about version numbers. I've switched to calver just to avoid thinking about this (yeah, I should have a deprecation policy but... my code just isn't that tidy yet).

[altendky]

Oh, as to using some interface tool or not, I see it as an optional extra like type hints. It isn't needed for things to work, it can provide some more clarity to both library and application developers.

[ebroecker]

... the interface for the formats is quite simple:

• load()
• dump()

using zope for just defining load and dump seems a bit overengineered to me.
Maybe I'm wrong. What do you have in mind while using something like zope?
How could it look like, perhaps I just did not understand the concept yet.

About versions:
My idea was like having a 1.x branch which is py2 compatible and a 2.x+ branch which is only py3.
I'm also OK with merging these things above in pre-1.0, but if so, I think, we should support py2...
I'm also OK with switching to calver or whatever, maybe if we get automatic releases work (related to #291)

[altendky]

Classes were what was in my head. I didn't bother to think through it recently though. But, the act of even trying to define the interface sometimes is useful. Do we really want all this stuff on one class? Anyways, my recent response wasn't to say I thought defining interfaces formally should be a priority.

[Funth0mas]

Just my thoughts... Convert formats from module level functions to the classes would be useful for getting rid of the global keyword in yaml, xlsx, sym and arxml formats and also of the lazy _init_yaml in the yaml format. I believe that the py2 support is no pain for such refactoring. Pain is to well define it...
Similar for the code style cleanup, it should not break py2.
Maybe I'm wrong but even if you drop py2, you can't use new features coming in py 3.6, 3.7, so currently dropping py2 shouldn't be that high prio. (I understand "drop py2" as "find and remove all workarounds as future library or sys.version conditions)

BTW, @ebroecker , the task "loadb for loading any format from a byte sequence" above is still valid, or already resolved? Thanks.