pydifact icon indicating copy to clipboard operation
pydifact copied to clipboard

AssertionError

Open alexvkaam opened this issue 1 year ago • 6 comments

hi

first of all, thank you for this library, it is briliant :) I ran into 1 issue, sometimes I get several messages wrapped together into 1 file, so like this:

UNB UNH UNT UNZ UNB UNH UNT UNZ

which works fine however sometimes these bundles also contain the UNA, now 1 UNA at the top is fine but if i'ts pure/raw messages that get bundled then I see an UNA infront of each UNB

UNA UNB UNH UNT UNZ UNA UNB UNH UNT UNZ

and that will cause an AssertionError.

it’s easily fixed by removing them before loading in or splitting them per UNA but it took me some time to figure out why some files worked and some did not. So I figured I mention it.

Regards,

alexvkaam avatar Aug 28 '24 08:08 alexvkaam

Hm. While it may be "easy" to fix, I'm not sure if this is concern of pydifact. Because the specification says that one UNA header must be present. If you wipe out the UNA header of a concatenated second message, the separator characters could be different from the ones the first message uses.

Yes I know, almost all messages worldwide use the same characters - but that's the problem of deduction: Just because you never saw a black swan, it doesn't mean they don't exist.

So it would be really bad to just "ignore" all following UNAs and skip them.

The only way I could think of solving this, is: let pydifact accept concatenated EDIFACTs - and treat each one as separate one.

And, just to ask, why do you get more than one messages wrapped into one file? Who does this? ;-) Wouldn't it be better to fix the real bug and stop him doing this?

nerdoc avatar Sep 02 '24 21:09 nerdoc

I think, as we now solved an edge case as well by closing #76, I might change that behaviour too . The best software is fault tolerant, even if this is not part of the spec.

My idea is: allow more UNA headers and parse each message, but spit out a warning.

Pydifact normally produces a list of Segment() objects.

When parsing 2 (or more) messages, it would have to spit out a list of list of Segments. This would break the API, as the user expects the Segments in the first level.

And as this lib is already in use under production, I don't want to change the API as it would break existing software, and might thus cause the apocalypse :laughing:

@alexvkaam ideas?

  • there could be an explicit parser call for "multi" message files.
  • ...?

nerdoc avatar Oct 21 '24 11:10 nerdoc

Good Evening,

first of all my apologies for not responding, your first reply was send on the first day of my holiday and it got snowed under once I came back.

To be honest I totally agree with you on your first post, it's against spec. It was simply the "AssertionError." that was very unclear.

So I personally think you should give a more user friendly error if there is more then 1 UNA. "Multiple UNA per input file not supported"

If you want to be super user friendly then you could simply cut the input file you get per UNA, if its all according to spec you end up with 1 block, if not you end up with multiple blocks, then simply push each block through your normal code, that way there is not need for a 2nd call. So basically a small pre-step in your Interchange.from_file ?

Regards,

alexvkaam avatar Oct 21 '24 15:10 alexvkaam

Sure, but how to return the parsed two blocks? Because the user expects a list of segments, and not a list of a list of segments...

nerdoc avatar Oct 22 '24 06:10 nerdoc

True, then a new call or just a proper error message i would say. The user can then just split the input before doing the calls

Regards

Op di 22 okt. 2024 08:24 schreef Christian González < @.***>:

Sure, but how to return the parsed two blocks? Because the user expects a list of segments, and not a list of a list of segments...

— Reply to this email directly, view it on GitHub https://github.com/nerdocs/pydifact/issues/73#issuecomment-2428359573, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZBQPCITV5S7RBAFO7N6FF3Z4XVTBAVCNFSM6AAAAABNHWOD72VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRYGM2TSNJXGM . You are receiving this because you were mentioned.Message ID: @.***>

alexvkaam avatar Oct 22 '24 08:10 alexvkaam

OK, that's fine for me, I'll implement that.

nerdoc avatar Oct 22 '24 14:10 nerdoc

Ok. I now just implemented a simple EDISyntaxError raise when a second UNA header was detected.

@alexvkaam please tell me if this is not what you wanted.

Oh, and sorry for the delay ;-)

nerdoc avatar May 03 '25 19:05 nerdoc

Hi, that seems the best way to handle it. Thank you

Op za 3 mei 2025 21:30 schreef Christian González @.***

:

nerdoc left a comment (nerdocs/pydifact#73) https://github.com/nerdocs/pydifact/issues/73#issuecomment-2848773474

Ok. I now just implemented a simple EDISyntaxError raise when a second UNA header was detected.

@alexvkaam https://github.com/alexvkaam please tell me if this is not what you wanted.

— Reply to this email directly, view it on GitHub https://github.com/nerdocs/pydifact/issues/73#issuecomment-2848773474, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZBQPCMXQ3JPCEDWEYEPHA324UKORAVCNFSM6AAAAAB4MBGSOCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNBYG43TGNBXGQ . You are receiving this because you were mentioned.Message ID: @.***>

alexvkaam avatar May 04 '25 04:05 alexvkaam