useblocks / sphinx-needs

Adds needs/requirements to sphinx
https://sphinx-needs.readthedocs.io/en/latest/index.html
MIT License
211 stars 66 forks source link

Namespace NeedsInfo data (and omit unset fields) #997

Open chrisjsewell opened 1 year ago

chrisjsewell commented 1 year ago

During work on #987, and in particular defining the NeedsInfoType schema, it was

  1. incredibly difficult to track down the genesis of all the data fields (and unserstand what they mean, and
  2. very difficult to define a clear schema, when dynamic fields are mixed with static ones
  3. It appears there is also a good chance for key clashes

Take for example the first need generated in the needs.json.

As shown in the second example, it would be much clearer to namespace the dynamic values.

Note, for back-compatibility, one may want to "collapse" the namespaces when using them as a context in https://github.com/useblocks/sphinx-needs/blob/a16fb72d44b2c9f65c4065e386e2ad37d0a25870/sphinx_needs/filter_common.py#L280

It is also of note, that currently unset fields are given defaults. It would reduce the memory footprint of the needs data greatly, if unset fields were simply omitted. This work require work in the code base to account for missing keys.

{
  "ACT_ANALYSIS": {
    "amount": "",
    "another_option": "",
    "arch": {},
    "author": "",
    "avatar": "",
    "blocks": [],
    "blocks_back": [],
    "checks": [],
    "checks_back": [],
    "closed_at": "",
    "comment": "",
    "completion": "",
    "config": "",
    "constraints": [],
    "constraints_passed": null,
    "constraints_results": {},
    "content_id": "ACT_ANALYSIS",
    "created_at": "",
    "delete": null,
    "description": "",
    "docname": "directives/needsequence",
    "doctype": ".rst",
    "duration": "",
    "ends_with": [],
    "ends_with_back": [],
    "external_css": "external_link",
    "external_url": null,
    "full_title": "Analysis issue",
    "github": "",
    "has_dead_links": "",
    "has_forbidden_dead_links": "",
    "hidden": "",
    "hours": "",
    "id": "ACT_ANALYSIS",
    "id_complete": "ACT_ANALYSIS",
    "id_parent": "ACT_ANALYSIS",
    "id_prefix": "",
    "image": "",
    "is_external": false,
    "is_modified": false,
    "is_need": true,
    "is_part": false,
    "jinja_content": null,
    "layout": "",
    "links": ["USER_B"],
    "max_amount": "",
    "max_content_lines": "",
    "modifications": 0,
    "my_extra_option": "",
    "params": "",
    "parent_need": "",
    "parent_needs": [],
    "parent_needs_back": [],
    "parts": {},
    "post_template": null,
    "pre_template": null,
    "prefix": "",
    "query": "",
    "section_name": "needsequence",
    "sections": ["needsequence"],
    "service": "",
    "signature": "",
    "specific": "",
    "starts_after": [],
    "starts_after_back": [],
    "starts_with": [],
    "starts_with_back": [],
    "status": null,
    "style": "yellow_border",
    "tags": [],
    "target_id": "ACT_ANALYSIS",
    "template": null,
    "tests": [],
    "tests_back": [],
    "title": "Analysis issue",
    "triggers": [],
    "triggers_back": [],
    "type": "action",
    "type_name": "Action",
    "unit": "",
    "updated_at": "",
    "url": "",
    "url_postfix": "",
    "user": "",
    "value": ""
  }
}
{
  "ACT_ANALYSIS": {
    "user_options": {
        "my_extra_option": "",
        "another_option": "",
        "author": "",
        "amount": "",
        "comment": "",
        "hours": "",
        "image": "",
        "config": "",
        "github": "",
        "unit": "",
        "value": ""
    },
    "links": {
      "links": ["USER_B"],
      "blocks": [],
      "checks": [],
      "ends_with": [],
      "starts_after": [],
      "starts_with": [],
      "tests": [],
      "triggers": []
    },
    "back_links": {
      "blocks": [],
      "checks": [],
      "ends_with": [],
      "starts_after": [],
      "starts_with": [],
      "tests": [],
      "triggers": []
    },
    "services": {
      "github": {
        "avatar": "",
        "closed_at": "",
        "created_at": "",
        "max_amount": "",
        "max_content_lines": "",
        "id_prefix": "",
        "query": "",
        "service": "",
        "specific": "",
        "type": "action",
        "updated_at": "",
        "url": "",
        "user": "",
      },
      "open_needs": {
        "max_content_lines": "",
        "id_prefix": "",
        "params": "",
        "prefix": "",
        "query": "",
        "url": "",
        "url_postfix": "",
      }
    },
    "arch": {},
    "completion": "",
    "constraints": [],
    "constraints_passed": null,
    "constraints_results": {},
    "content_id": "ACT_ANALYSIS",
    "delete": null,
    "description": "",
    "docname": "directives/needsequence",
    "doctype": ".rst",
    "duration": "",
    "external_css": "external_link",
    "external_url": null,
    "full_title": "Analysis issue",
    "has_dead_links": "",
    "has_forbidden_dead_links": "",
    "hidden": "",
    "id": "ACT_ANALYSIS",
    "id_complete": "ACT_ANALYSIS",
    "id_parent": "ACT_ANALYSIS",
    "is_external": false,
    "is_modified": false,
    "is_need": true,
    "is_part": false,
    "jinja_content": null,
    "layout": "",
    "modifications": 0,
    "parent_need": "",
    "parent_needs": [],
    "parent_needs_back": [],
    "parts": {},
    "post_template": null,
    "pre_template": null,
    "section_name": "needsequence",
    "sections": ["needsequence"],
    "signature": "",
    "status": null,
    "style": "yellow_border",
    "tags": [],
    "target_id": "ACT_ANALYSIS",
    "template": null,
    "title": "Analysis issue",
    "type_name": "Action",
  }
}
PhilipPartsch commented 1 year ago

I do not understand,if the issue only want to change the needs.json output or even the internal representation. Any way: it is cool that the needs.json has the same structure as the internal. So a needs.json can be used as blue print for implementation of filter and functions or even extensions. If you try to overcome clashs, we do need name spaces even in rst. And i belive this is verbose for the author of a rst file.

What do you think?

chrisjsewell commented 1 year ago

Any way: it is cool that the needs.json has the same structure as the internal.

Heya, The problem is that there is no structure 😅 The information stored in the needs info, mixes static (known a priori) and dynamic (set by the user) data fields, in a manner that makes it very difficult to work with programmatically and prone to bugs, due to their being no type safety of the data (see for example #995)

chrisjsewell commented 1 year ago

we do need name spaces even in rst. And i belive this is verbose for the author of a rst file.

This is not the case, the data is already "naturally" name-spaced. No change is required for user input

chrisjsewell commented 1 year ago

Happy to talk offline with you about this @PhilipPartsch In the mean time you may want to check out https://github.com/useblocks/sphinx-needs/blob/64ce426fa60543b388196a349bbf1d4d99d15c86/sphinx_needs/data.py#L80 which I painstakingly deciphered from the current code 😅

My question would be, could you now (without the schema) actually look at the ~85 fields in the above example, and tell me what they all mean and where they come from? I think currently the data.json (and internal representation) is very difficult to understand in its entirety, particularly for new users and developers. Having a clear schema will make this much more accessible