Skip to content
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

Adapt data-structure(s) & API signature(s) of Karabo #609

Open
Lukas113 opened this issue Aug 12, 2024 · 1 comment
Open

Adapt data-structure(s) & API signature(s) of Karabo #609

Lukas113 opened this issue Aug 12, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@Lukas113
Copy link
Collaborator

Lukas113 commented Aug 12, 2024

According to my knowledge, we could improve some data-structures & API's of Karabo:

  • Define common API-terminology used for variable-names through Karabo in a document and apply this on the repo.
  • Change all phase_center args to tuple[float, float] and unify signatures (e.g. not take ra-dec as separate args anywhere where both are needed). The reason is simply because a tuple can have a fixed length, where a list (or any Sequence) is not defined as strictly.
  • Replace evil mutable-default args (e.g. SkyModel.setup_default_wcs phase_center).
  • Add more kw-only args, especially to avoid boolean-trap anti-pattern. Usually, the first argument(s) are data and/or path args, which are not necessarily kw-only args. However, customization args for that particular data should be (as far as I know) kw-only to improve readability & maintainability (usually helps to be backward compatible) of code.
  • Introduce unit-suffix to unit-args if units are not base SI-unit, e.g. deg, arcsec, mhz, etc. if it's not a astropy.units.quantity.Quantity, but just int or float.
  • Adapt typing to accept also e.g. Sequence and not only list and/or tuple wherever it makes sense. This is also true for some other collections.abc types.
  • Support str and pathlib.Path as path input for user-api args. Our API currently is not consistent in this domain.
  • I'm sure there are other cases (e.g. list should be replaced with tuple to improve the data-structures and signatures of Karabo's API.
  • Probably the biggest change is to think about a Public API to get to v1.0.0. Expose each function/class/module which are public and prefix-underscore each non public function/class/module. This may also include some API changes itself. However, if this is a change which is too large, it can be done in a different PR. This would contribute to issue Karabo Versioning #614 .
@Lukas113 Lukas113 added the enhancement New feature or request label Aug 12, 2024
@Lukas113
Copy link
Collaborator Author

This shouldn't take too long I think. However, this will most likely lead to API-breaking changes.

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

No branches or pull requests

1 participant