nerdocs / pydifact

A python EDIFACT library.
MIT License
156 stars 45 forks source link

Unused property delimiters of Interchange class. #70

Closed airguru closed 3 weeks ago

airguru commented 9 months ago

I was trying to serialize an Interchange with custom delimiters property, but it did not seem to have any effect. I was always getting UNA:+,? '. Turns out, in Interchange constructor:

    def __init__(
        self,
        sender: str,
        recipient: str,
        control_reference: str,
        syntax_identifier: Tuple[str, int],
        delimiters: Characters = Characters(),
        timestamp: datetime.datetime = None,
        *args,
        **kwargs
    ):
        super().__init__(*args, **kwargs)
        self.sender = sender
        self.recipient = recipient
        self.control_reference = control_reference
        self.syntax_identifier = syntax_identifier
        self.delimiters = delimiters
        self.timestamp = timestamp or datetime.datetime.now()

the self.delimiters property is never actually used. In serialize() method inherited from AbstractSegmentsContainer, in reality self.characters is passed into Serializer class. I believe this is just misnaming and the delimiters property in Interchange class should be renamed to characters.

mj0nez commented 9 months ago

Did you use the pypi version? I think #59 fixed this. Could you try the latest version from main?

airguru commented 9 months ago

Yes, i just used pypi version. Pip freeze gives me pydifact==0.1.6 . From looking at #59, I see that likely adding characters parameter to the interchange constructor could work (because it would get passed to AbstractSegmentsContainer. Is that not part of the pypi version?

However, the unused self.delimiters property is till there.

mj0nez commented 9 months ago

Unfortunately, it was merged after the release of 0.1.6 and hasn't made it to pypi yet.

nerdoc commented 9 months ago

I just released a new version 0.1.7 to pypi with the current version in git master. @airguru could you please make a pip install -U pydifact and test if it works now? BTW. I finally dropped Python 3.5 (and 3.6, 3.7) support, as they are basically unmaintained now.

airguru commented 9 months ago

It seems to work now when passing characters arg into Interchange constructor. The delimiters can now be really deleted :)

nerdoc commented 3 weeks ago

@airguru you mean you pass characters into the Parser() constructor? I deleted the delimiters parameter - it's really unused.