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

Create Markdown Operator Docs #2019

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from
Draft

Conversation

ansMHanmer
Copy link

@ansMHanmer ansMHanmer requested a review from PProfizi January 17, 2025 20:57
@ansMHanmer ansMHanmer self-assigned this Jan 17, 2025
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.53%. Comparing base (05c565c) to head (e84339c).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2019      +/-   ##
==========================================
- Coverage   86.54%   85.53%   -1.02%     
==========================================
  Files          90       90              
  Lines       10275    10275              
==========================================
- Hits         8893     8789     -104     
- Misses       1382     1486     +104     

Copy link
Contributor

@PProfizi PProfizi left a comment

Choose a reason for hiding this comment

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

Hi @ansMHanmer, just a few suggestions.

Also, do you know if we could set some of the properties as Markdown metadata?
I think this is what we will use to allow for tagging and filtering of operators in the final documentation.
Not sure if we want to use MultiMarkdown Metadata or YAML Front Matter or something else.

@ansMHanmer
Copy link
Author

ansMHanmer commented Feb 11, 2025

Hi @ansMHanmer, just a few suggestions.

Also, do you know if we could set some of the properties as Markdown metadata? I think this is what we will use to allow for tagging and filtering of operators in the final documentation. Not sure if we want to use MultiMarkdown Metadata or YAML Front Matter or something else.

Thanks for the suggestions, working away at them. Yes, we can set some of the properties as Markdown metadata, and it's probably best to use YAML Front Matter based on some research, so I'll go with that. If we find we need to use MultiMarkdown I can easily switch.

@PProfizi - which properties would you want to see in the metadata? I was about to go implement, but figured it would be best to ask what the desired outcome was first 😄

@PProfizi
Copy link
Contributor

PProfizi commented Feb 17, 2025

Hi @ansMHanmer, just a few suggestions.
Also, do you know if we could set some of the properties as Markdown metadata? I think this is what we will use to allow for tagging and filtering of operators in the final documentation. Not sure if we want to use MultiMarkdown Metadata or YAML Front Matter or something else.

Thanks for the suggestions, working away at them. Yes, we can set some of the properties as Markdown metadata, and it's probably best to use YAML Front Matter based on some research, so I'll go with that. If we find we need to use MultiMarkdown I can easily switch.

@PProfizi - which properties would you want to see in the metadata? I was about to go implement, but figured it would be best to ask what the desired outcome was first 😄

So we are working on actually adding some properties we eventually will want to see as filterable/searchable metadata, but for a first PoC we can use plugin to demonstrate it is handled. Probably the license property of the operator should be metadata we can filter with. Also, the category is used in the current doc to organize operators, so I think it should be included in the metadata of the operator to allow filters/classifiers to have access to that.

The end-goal would be for example to allow a user to look for operators with license equal to None, in category invariant, for plugin core.

@ansMHanmer
Copy link
Author

@PProfizi i've added in the metadata as requested.

On another note - I've noticed that quite a few operators were failing to retrieve their info properly because of some apparently missing data.

When trying to retrieve spec.config_specification for many operators, I am met with:

Ansys Path: C:\users\mhanmer\dpf\ansys\dpf\server_2025_1_pre0\
Server Info: {'server_ip': '', 'server_port': None, 'server_process_id': 27724, 'server_version': '9.0', 'os': 'nt', 'path': 'C:\\users\\mhanmer\\dpf\\ansys\\dpf\\server_2025_1_pre0\\'}
Server Context: Server Context of type LicensingContextType.premium with no xml path
Server Config: Server configuration: protocol=InProcess
Server version: 9.0
Traceback (most recent call last):
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.ci\generate_operators_doc.py", line 160, in <module>
    main()
    ~~~~^^
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.ci\generate_operators_doc.py", line 156, in main
    generate_operator_doc(server, operator_name, args.include_private)
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.ci\generate_operators_doc.py", line 120, in generate_operator_doc
    operator_info = fetch_doc_info(server, operator_name)
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.ci\generate_operators_doc.py", line 60, in fetch_doc_info
    for configuration_key in spec.config_specification:
                             ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.venv\Lib\site-packages\ansys\dpf\core\operator_specification.py", line 452, in config_specification
    num_options = self._api.operator_specification_get_num_config_options(self)
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.venv\Lib\site-packages\ansys\dpf\gate\generated\operator_specification_capi.py", line 228, in operator_specification_get_num_config_options
    raise errors.DPFServerException(sError.value)
ansys.dpf.gate.errors.DPFServerException: C-layer: Required type is incorrect

I have added some handling around it so the docs generation can continue, but we're left with very incomplete docs in the cases where this error comes up. Perhaps if there is a certain amount of missing information we log an error and skip the doc for that particular operator.

For you to see, it's probably easiest just to try the latest version of the script on your end to see if you can reproduce and provide comment.

@PProfizi
Copy link
Contributor

@PProfizi i've added in the metadata as requested.

On another note - I've noticed that quite a few operators were failing to retrieve their info properly because of some apparently missing data.

When trying to retrieve spec.config_specification for many operators, I am met with:

Ansys Path: C:\users\mhanmer\dpf\ansys\dpf\server_2025_1_pre0\
Server Info: {'server_ip': '', 'server_port': None, 'server_process_id': 27724, 'server_version': '9.0', 'os': 'nt', 'path': 'C:\\users\\mhanmer\\dpf\\ansys\\dpf\\server_2025_1_pre0\\'}
Server Context: Server Context of type LicensingContextType.premium with no xml path
Server Config: Server configuration: protocol=InProcess
Server version: 9.0
Traceback (most recent call last):
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.ci\generate_operators_doc.py", line 160, in <module>
    main()
    ~~~~^^
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.ci\generate_operators_doc.py", line 156, in main
    generate_operator_doc(server, operator_name, args.include_private)
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.ci\generate_operators_doc.py", line 120, in generate_operator_doc
    operator_info = fetch_doc_info(server, operator_name)
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.ci\generate_operators_doc.py", line 60, in fetch_doc_info
    for configuration_key in spec.config_specification:
                             ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.venv\Lib\site-packages\ansys\dpf\core\operator_specification.py", line 452, in config_specification
    num_options = self._api.operator_specification_get_num_config_options(self)
  File "C:\Users\mhanmer\Repositories\GitHub\ansys\pydpf-core\.venv\Lib\site-packages\ansys\dpf\gate\generated\operator_specification_capi.py", line 228, in operator_specification_get_num_config_options
    raise errors.DPFServerException(sError.value)
ansys.dpf.gate.errors.DPFServerException: C-layer: Required type is incorrect

I have added some handling around it so the docs generation can continue, but we're left with very incomplete docs in the cases where this error comes up. Perhaps if there is a certain amount of missing information we log an error and skip the doc for that particular operator.

For you to see, it's probably easiest just to try the latest version of the script on your end to see if you can reproduce and provide comment.

Thanks @ansMHanmer I'll have a look at the issue.

@PProfizi
Copy link
Contributor

I've made a MRE which lists failing operators, I'll investigate the error and try to fix it.

import ansys.dpf.core as dpf
from ansys.dpf.gate.errors import DPFServerException


failing = []
for operator_name in dpf.available_operator_names():
    op = dpf.dpf_operator.Operator(operator_name)  # "mapdl::cms_deprecated::EEL"
    spec = op.specification
    try:
        _ = spec.config_specification
    except DPFServerException as e:
        failing.append(operator_name)
print(sorted(failing))
print(len(failing))
assert len(failing) == 0

@PProfizi
Copy link
Contributor

Hi @ansMHanmer, turns out the issue is that these operators do not have a specification to start with.
Until now we have returned an empty Specification for those, but then any query would fail, and this particular one does not check if the spec is empty. I can fix that with one line, but the question is whether we should not rather throw already when requesting the spec for an operator which does not have one, instead of returning an empty Specification. What do you think?

@ansMHanmer
Copy link
Author

Hi @ansMHanmer, turns out the issue is that these operators do not have a specification to start with. Until now we have returned an empty Specification for those, but then any query would fail, and this particular one does not check if the spec is empty. I can fix that with one line, but the question is whether we should not rather throw already when requesting the spec for an operator which does not have one, instead of returning an empty Specification. What do you think?

Thanks for taking a look @PProfizi - and the change you made in #2107 seems to work well on my side!

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.

2 participants