fixes #49 and black format, fixed typing issues

[Dronakurl]

• I fixed the "get started" issue :-)

• I use pyright and ruff for code linting and formatting. I changed a few things in the code, so it adheres to the standards applied by these tools. Most of the changes are just format, but the linter also found minor errors like this (__all__ should contain names not the objects):

-__all__ = [read, SddsFile, write, __version__]
+__all__ = ["read", "SddsFile", "write", "__version__"]

• If you want to use other standards, I recommend using poetry and a pyproject.toml. If you want to, I can add it to the repo.

[JoschD]

Hello @Dronakurl ,

thank you very much for your contribution!
I appreciate especially, that you added a test to assure your fix of #49 works as expected :) .
I see you cleaned up the code a lot and fixed some (possible?) issues here and there as well, some changes of which I have to admit I do not immediately understand.

E.g. the additional get_key_value_string() functions (which btw have a typo in the warnings)
and the __getitem__ in the __write _data.

Could you please write an entry into the changelog, mentioning user-affecting changes?
Again, thank you very much!!

Cheers,

Josch

[JoschD]

Btw. I think the tests might be failing because of the SECRETS we have used in the workflows and because this is a PR stemming from a fork. I will try to figure out how to handle that best

[fsoubelet]

Hi @Dronakurl, thanks for the contribution! Always happy to see this gathers some interest from outside. I'm pointing out the following, on top of what @JoschD mentionned.

Regarding your suggestions, uing a pyproject.toml file is on my list of things to do. I will migrate our repos in due time, thanks for the offer.

While I have used it in the past for personnal projects, we will not be using Poetry in PyLHC repositories. There are several reasons for this decision, mainly that Poetry decides to do its own thing too often. I'll list some strong ones here to point to this comment later should I need.

For instance:

• Poetry automatic version constraints (e.g. when using poetry add) are extremely harsh, and cause dependency resolving issues down the line, sometimes for entire small ecosystems.
• Poetry strictly enforces semantic versioning and while we use it, I don't like the idea of a tool that prevents you from making such important choices. A versioning scheme is bigger than a codestyle choice.
• Poetry does not respect the standardized PEP 621 on project metadata specification, instead making up its own section and syntax.
• Poetry does not respect the standardized PEP 631 on dependency specification, instead making up its own section and syntax.
• Poetry does not respect the standardized PEP 518 on build systems specification, instead making up its own section and syntax.

The above (and more) mean we would have to force everyone to use Poetry in order to contribute to our projects, and that our projects would not comply with the Python packaging standards. I don't fancy that.

If anything, I would prefer using a (non-mandatory) tool such as Hatch which sticks very close to the standards.

[Dronakurl]

Hi, I updated the PR. That type hint is tricky. pyright still showed type errors when assigning a Definition to a Parameter, Array, Column, despite the fact, that I specifically checked whether it was that subclass. So type: ignore was the best option.

I also fixed a DeprecationWarning.

DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)

As for the Changelog: There are no significant user facing changes, but I added an entry as requested

[fsoubelet]

I also forgot: we should bump the version to 0.4.1. @Dronakurl would you mind doing so, in the package's __init__.py?

[Dronakurl]

Hi, is there something missing with this PR? I am a bit lost, what to do with those checks. Can you please help?

[JoschD]

Hey @Dronakurl ,

there is nothing to do about the checks for you. We need to move the branch from your repo to this one and then they can run.
The reason I have not done that yet, is that I was still waiting for you to answer the questions I posed in my first comment, that is what is the purpose of

• get_key_value_string() in Data and Description, if it's not implemented anyway?
• __getitem__ in __write _data? I do not see how this is used.

Cheers,

Josch

[Dronakurl]

Hi Josch, Ah, I missed these questions. As for `get_key_value_string`: If this method is not defined in the parent class, the following function shows diagnostics (from pyright) ```python def _sdds_def_as_str(definition: Union[Description, Definition, Data]) -> str: return f"{definition.TAG} {definition.get_key_value_string()} &end\n" ``` The `__get_item__` is a remnant of a botched attempt to solve the typing in this function more elegantly. I deleted it. Best regards Dronakurl Am Do., 1. Feb. 2024 um 11:26 Uhr schrieb JoschD ***@***.***>:

…

Hey @Dronakurl <https://github.com/Dronakurl> , there is nothing to do about the checks for you. We need to move the branch from your repo to this one and then they can run. The reason I have not done that yet, is that I was still waiting for you to answer the questions I posed in my first comment, that is what is the purpose of - get_key_value_string() in Data and Description, if it's not implemented anyway? - __getitem__ in __write _data? I do not see how this is used. Cheers, Josch — Reply to this email directly, view it on GitHub <#59 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASKZU4DOTOKLOTPKERBTTILYRNUTZAVCNFSM6AAAAABAQMOVD2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRQHE4TGMZVHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[JoschD]

Hey @Dronakurl , I am very sorry that this took so long. I was very busy throughout January and then also on holidays. Your effort in this is highly appreciated and I promise that future PRs will be handled quicker :)

[fsoubelet]

Happy to merge. Thanks again @Dronakurl :)