nerdocs / pydifact

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

Refactor the documentation #71

Closed aptiko closed 3 weeks ago

aptiko commented 5 months ago

Hello,

I want to work on this project, and the first thing I'd like to do is improve the documentation. Is this OK? I have a question already, and as I read the existing documentation (and code) carefully, I'm sure I'll have lots more questions, so I'll be asking them here.

First question: Is AbstractSegmentsContainer meant to be used by the user (i.e. the developer that uses pydifact), or only by the developer (i.e. the developer that develops pydifact itself)? My opinion (and standard practice in projects such as Django) is that if it's an internal, we should not bother the user.

aptiko commented 5 months ago

As I worked, the question partly answered itself. Yes, the user needs to know about the AbstractSegmentsContainer methods and attributes that are inherited by the subclasses. (However I'm not certain he needs to know the mechanics of inheritance, such as HEADER_TAG.)

aptiko commented 5 months ago

I did some work, available at my fork and compiled at a temporary place online. So far I've only worked on AbstractSegmentsContainer. Please tell me your opinion before I continue.

nerdoc commented 5 months ago

Hi @aptiko, this looks great. The docs definitely need some love. You can certwinly ioen a PR for this, and I'd be happy to merge it. Just make sure code you write is tested. And about AbstractSegmentsContainer: yes this is meat to be used by the developer to subclass "real" SegmentsContainers, which then are meant to be used by pydifact's users. So, the AbstractSC is "internal", as the AbstractBaseUser in Django. But as long as there are some needed by users, you'd have to make subclasses as user yourself. But if you ask me, I'd make it marked internal, but document the methods and attributes, as you said.

nerdoc commented 5 months ago

Oh, and HEADER_TAG definitely isn't something the user should change. In the best case, the user should use the subclass of AbstractSegmentsContainer. So it could be named _HEADER_TAG, but this would look a bit uglier IMHO.

nerdoc commented 2 months ago

Hey @aptiko! How's your work going on?

aptiko commented 2 months ago

@nerdoc Thanks for reminding me. Unfortunately it seems I won't work much on EDI, at least not at the moment. So I submitted my half-finished job.

nerdoc commented 3 weeks ago

closed by #74 Rest of documentation is TBD, but for now, we leave it. PRs welcome.