Skip to content

Typing #80

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Typing #80

wants to merge 17 commits into from

Conversation

tammoippen
Copy link

Hi @nerdoc,

I am using your project and would like to improve on the typing situation. Along with that, I

  • removed SegmentCollection, FileSourcableMixin and UNAHandlingMixin, as that improved typing a lot (and they were to be dropped in v0.2 anyway)
  • increased the minimum python version to 3.9, as 3.8 is EOL since September 2024. (https://devguide.python.org/versions/)

Best and thank you for your effort.

@lord-haffi
Copy link

lord-haffi commented Apr 4, 2025

Actually stumbled across this issue right now; funny that you just opened a PR to support proper typing ^^
Big thumb up from me 👍

@@ -364,7 +364,7 @@ def __init__(
sender: Element,
recipient: Element,
control_reference: Element,
syntax_identifier: Element,
syntax_identifier: tuple[str, int],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to move here to an even more specific Element, but tuple[str, int] is ok for me for the moment.

@nerdoc
Copy link
Member

nerdoc commented Apr 8, 2025

Great, thanks for the improvements, that's a thing I always wanted, too.
Since we are dropping py3.8 support now (god thanks), I would even go farther and remove "typing" imports, and all the List, Tuple etc. stuff.

from typing import List, Tuple

foo: List[int]
bar: Tuple[str, int]
baz: Type[int]

could be written much easier as

foo: list[int]
bar: tuple[str, int]
baz: type[int]

with Python builtins.

The only things that remain (until python 3.10) are the Union and Optional annotations (which I hate too).

pydifact has been proven stable for a longer time now, and people who are using python 3.8 because their company thinks it must stay in 2024, can use earlier pydifact versions too IMHO, but I'd like to go onwards.

So why not drop 3.9 support too, security EOL is 31 Oct 2025, that's almost tomorrow.

And with 3.10 we could write int | None instead of the bulky Optional[int]

What do you say? I'm fine with it, so If you want you can just add this as additional commit and remove all the ballast of Optional, Union, and python 3.9 dependencies in .github and .travis.yml files.
(I did that for 3.8 in 3 separate commits online on Github.com, that's why it's 3...)

@@ -80,7 +80,7 @@ def test_get_segment_w_predicate():

def test_split_by():
def _serialize(collections: Iterable[RawSegmentCollection]) -> List[List[str]]:
lst = []
lst: list[str] = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is python 3.10 anyway...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://peps.python.org/pep-0585/ is available since 3.9.

@tammoippen
Copy link
Author

Hi @nerdoc, I adjusted for python 3.10. Best, Tammo

@nerdoc
Copy link
Member

nerdoc commented Apr 8, 2025

looks very good, I'll check the code a bit, as I in parallel made some changes in other branches that go into the same direction, but differ a bit. Precisely, I thought about creating a factory not only for segments, but also for data elements - and classes for them to make more "typed" parsing. But that's maybe too much memory it will consume.
And for my part, I am working on the support of more than one EDIFACT version - ATM it's 3/4 which is supported, but in my case I need to support version 1 (here in Austria, we live in the 90ies, in medicine).

Side note: I am looking for an official, machine readable document that describes the edifact format, so pydifact can parse it and validate the Segments it reads in against it. I have already written to the UN, but I get no answer besides the documents on unece.org - which are written for humans. No XSD schemata, no JSON formats, nothing. Just plain prosa, and tables. Maybe I'll open an issue for that, if anyone wants to join there: #81

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants