Implement Message and Interchange high-level containers

[JocelynDelalande]

This PR fixes #19

Limitations

• Functional groups are not implemented, I do not plan to implement them soon myself. (their use is optional, and I never used/seen them in the wild)
• Syntax / consistency checks are kinda lousy, IMHO this is not the time to handle that any further : that could take place in a more general work on Segment specialization/validation (huge work ahead, if so).

TODO

(things I want to do before merging)

• Accept optional segments in Message / Interchange headers
• Decide on API breakage strategy (see bellow)

API breakage strategy discussion

Compared to what we discussed in #19, I have two tasks pending :

• move from_file() to Interchange class
• move UNA handling to Interchange class

I wanted to stop and discuss here because, those changes are going to break previous API. I see three options:

Option 1 : prioritize compatibility

Let it as-is, more or less : the API is a bit less clean but backward compatibility is ensured

Option 2 : prioritize API cleanness

• Totally remove SegmentCollection identifier (optionally replace it with something throwing an exception with comprehensive message)
• if needed : Create a RawSegmentCollection that barely inherit AbstractSegmentsContainer and maintains the possibility to parse random segments buckets

Option 3 : progressive breakage

1. Publish a version following option 1, but add a deprecation warning when a SegmentCollection is instantiated
2. Publish a version (major one, because API breakage) following option 2

WDYT? As of now, I myself have a preference for Option 3

Any comments on the PR are welcome, even if this is WIP (design and most of the code is here already).

[nerdoc]

First, without I have seen the code: whow, I'm impressed. It's still hard to believe for me that anyone else finds this code useful for "production" already - because for me it's still in "eats your dog" mode. ;-)
But I'm more than happy that you seem to really dive into it and hel me in things I barely have time at the moment...
That's open source.

So, to the "options" - API stability is no issue ATM, as it really is meant "pre-alpha". But, nevertheless, it would be fair to choose option 3, for anyone who already uses the low level API. And it's GoodProgrammingPractice anyway. So. Option 3 should be the way to go.
I'll have a look at the code soon. Thanks for your valuable input.

[nerdoc]

One thing I see if I look at e.g. Message/Interchange: You use parameters of the __init__() function to initialize the created Interchange. But they are only the mandatory parameters of this segment. There are many more. we have to be extremely careful designing this API, as it is hard to be changed later, when its stable some time.
So, either it is necessary to implement ALL of the positions of a segment, including the optionals.
This maybe could be done by

class Message(AbstractSegmentsContainer):
    def __init__(
            self,
            reference_number: str,
            identifier: Tuple,
            *options
    ):

with gooooood documentation.
This way, optional elements could be added later without problems. and with out breaking API here.

And yes, there would be NO type whatsoever checking possibility when using str, tuples etc.
I must think about that (your opinions?) - I wanted to implement all the type checking functionality first - but maybe that's a bunch of work, doesn't help much in the first place and can be done later too. Maybe pydifact Is never going to be finished/stable if doing AllTheThingsCompletelyRightTheFirstTime(TM) ;-)

[nerdoc]

And I see you are using the SyntaxError Exception. I already created EDISyntaxError for this purpose - BUT - maybe it's not really a good name. And maybe Python's SyntaxError could be used for that too.

exception SyntaxError
Raised when the parser encounters a syntax error.

This says the Python docs - but I think we can use it here to.
So I will replace my EDISyntaxError with SyntaxError in one of the next commits.

[JocelynDelalande]

So I will replace my EDISyntaxError with SyntaxError in one of the next commits.

Or maybe just keep EDISyntaxError but make it inherit SyntaxError ?

[nerdoc]

Is there any recommendation to use SyntaxError only for Python syntax? I didn't find any. but naturally, I think using EDISyntaxError is the better way, so it's easier to distinguish. I'll make it inherit SyntaxError.

[JocelynDelalande]

So, either it is necessary to implement ALL of the positions of a segment, including the optionals.
This maybe could be done by

class Message(AbstractSegmentsContainer):
    def __init__(
            self,
            reference_number: str,
            identifier: Tuple,
            *options
    ):

with gooooood documentation.

Yes, that is on my TODO for this PR :). But I would prefer to use the *options stared format but rather a tuple/list : if for one reason or another we have to add an argument, it would totally break existing code.

[nerdoc]

"I would prefer to use the *options stared format" - you mean "not use"?
But then, better use a dict than a tuple. So let's use **options. Then It is even more compatible.
I see no big problem in `options neither. Because the only problem would be if a newer software uses an older pydifact lib which has not implemented "option x" - then it crashes. Using a tuple, or better a dict, then it is easier to check if a certain key is there. We have to check anyway - because blindly accessing a dict's key without knowing it is there does not work. and all these options are optional.

class Message(AbstractSegmentsContainer):
    def __init__(
            self,
            reference_number: str,
            identifier: Tuple,
            **options
    ):

[JocelynDelalande]

I'd prefer to allow adding arbitrary segment sections, rather than listing and handling all optional fields specifically. See my implementation in 5047490 WDYT ?

[JocelynDelalande]

(moreover, limiting the supported sections may force us to support all versions of the standard with their specificities, which is not a way I want to take as of now :-p).

[nerdoc]

Hm. I think you want to support the newest version (4). Due to my own requirements, and why I started pydifact, I need to support the v3. That's why I created the syntax directory with the v[1-3].py files... (they're stubs for now)
So yes, I'd like to support more than one versions, at least keep that in mind...
What is not necessary is to support them within one program. So IMHO it should be possible to use from pydifact.syntax.v4 import XYZSegment and use v4 then. But we would have to implement classes for Segments of each version then, or, if they differ few, put that class in syntax/common.py, handle the differences in the class and import that in the v3 and v4 namespace.

[JocelynDelalande]

Hm. I think you want to support the newest version (4). Due to my own requirements, and why I started pydifact, I need to support the v3. That's why I created the syntax directory with the v[1-3].py files... (they're stubs for now)
So yes, I'd like to support more than one versions, at least keep that in mind...
What is not necessary is to support them within one program. So IMHO it should be possible to use from pydifact.syntax.v4 import XYZSegment and use v4 then. But we would have to implement classes for Segments of each version then, or, if they differ few, put that class in syntax/common.py, handle the differences in the class and import that in the v3 and v4 namespace.

OK, thanks for the explanation, but within the scope of that PR, do my liberal implememantion of optional elements suits you ?

[nerdoc]

Oh, yes, sorry for the late answer. For this PR this is ok, if you don't mind if I change the API later...

[JocelynDelalande]

Oh, yes, sorry for the late answer. For this PR this is ok, if you don't mind if I change the API later...

Yes, sure. By the way, I'd be in favor of using semantic versioning : bump the major version when we break the API.

[JocelynDelalande]

"I would prefer to use the *options stared format" - you mean "not use"?

yes, sorry.

supporting arbitrary segments using dicts seems a bit inconsistent to me : as I am not going to document the allowed parameters for now (and they may vary from a version of edifact to another), telling users to pick arbitrary keys seems weird. So tuple for now, and we may switch to dict or named args later :)

[JocelynDelalande]

Adressed remaining issues and filled changelog. I think it is good to go :)

[nerdoc]

Regarding semantic versioning - definitively. Just startend with a 0.0. Version as it's pre-alpha. So there is no major version until stable, API can change any time... But to address the changes we could use minor versions for "major" changes too until 1.0.0

[JocelynDelalande]

Regarding semantic versioning - definitively. Just startend with a 0.0. Version as it's pre-alpha. So there is no major version > until stable, API can change any time... But to address the changes we could use minor versions for "major" changes too until 1.0.0

OK, not a big fan, but you are the maintainer, and I do not want wasting our time with endless discussion on versioning scheme, so let's go for that :).

OK to target SegmentCollection removal for v1.0 as I wrote in that PR, or before ?

[nerdoc]

OK, not a big fan, but you are the maintainer, and I do not want wasting our time with endless discussion on versioning scheme, so let's go for that :).

OK to target SegmentCollection removal for v1.0 as I wrote in that PR, or before ?

I maybe just misunderstood - I'm a BIG fan of SemVer. I just thought that UNTIL we reach 1.0.0 - API can change anyway. Because if not, I would have to change the major version with each API break - but the first break would then have to be 1.0.0 - which is not desirable - because 1.0.0 means stable.
See here, explaining:

Before publishing your first, useable version, you might find yourself incrementing the middle and the last digit to keep track of Alpha/Beta releases. Only once you’re ready for a proper, first release, should you start versioning from 1.0.0.

That's exactly what I want.
So, increase patch number with every release/change, and minor with greater breaks, UNTIL 1.0.0 - and then strictly semantic versioning...

[nerdoc]

OK to target SegmentCollection removal for v1.0 as I wrote in that PR, or before ?

I would remove that before. Noone should use that any more when 1.0 is near ;-)
Remember, this is (pre)alpha software ATM...

[JocelynDelalande]

Ok for everything, thanks for taking time explaining :). So if I got it correctly, I think my PR is ready with latest push.

• 0.1 with introduction of Interchange/Message containers
• 0.2 with API breakage

Sounds good to you ?

[JocelynDelalande]

Fixed a few leftovers (nitpicking) and a bug

[nerdoc]

Yes, this sounds fine!

[nerdoc]

Sorry for the long delay. Looks all good to me. Thank you very much for your input.

[JocelynDelalande]

No prob, and thanks :-).