Skip to content

3dbeacons api - #8253

Open
bienchen wants to merge 7 commits into
galaxyproject:mainfrom
bienchen:3dbeacons_api
Open

3dbeacons api#8253
bienchen wants to merge 7 commits into
galaxyproject:mainfrom
bienchen:3dbeacons_api

Conversation

@bienchen

Copy link
Copy Markdown
Contributor

FOR CONTRIBUTOR:

  • I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • Use of AI
    • The contribution is mostly AI generated
    • The contribution has been assisted by AI
  • License permits unrestricted use (educational + commercial)
  • This PR adds a new tool or tool collection
  • This PR updates an existing tool or tool collection
  • This PR does something else (explain below)

There are two labels that allow to ignore specific (false positive) tool linter errors:

  • skip-version-check: Use it if only a subset of the tools has been updated in a suite.
  • skip-url-check: Use it if github CI sees 403 errors, but the URLs work.

To request a review once your PR is ready, comment "please review" on the PR. This will
automatically apply the ready-for-review label if the PR is not a draft, all review
threads are resolved, and all CI checks have passed.

@bienchen

Copy link
Copy Markdown
Contributor Author

The linting step fails during checking:

.. WARNING (BioToolsValid): No entry 3d-beacons in bio.tools.
.. WARNING: remote_repository_url may be incorrect: expected it to end with a significant part of '/home/runner/work/tools-iuc/tools-iuc/tools/3dbeacons_api'

The first one seems to pop up because bio.tools is down atm. The second one I don't understand.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please add this file as <required_files> in the Galaxy tool.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Like this:

    <required_files>
        <include path="api_wrapper_3dbeacons.py"/>
    </required_files>

With no type so a normal path is assumed as default?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes :)

Comment thread tools/3dbeacons_api/3dbeacons_api.xml Outdated
<xref type="bio.tools">3d-beacons</xref>
</xrefs>
<requirements>
<requirement type="package" version="3.13">python</requirement>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
<requirement type="package" version="3.13">python</requirement>

requests will get you python

<when value="single">
<param name="qualifier" type="text" value="" optional="false"
label="Qualifier" help="ID/ AC to search for"/>
<param name="provider" type="select" optional="true"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you select multiple providers?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope, only a single one. I would love to see a real provider selection, but the 3D-Beacons API only uses the last one in the parameter list.

Comment thread tools/3dbeacons_api/3dbeacons_api.xml Outdated
<param name="provider" type="select" optional="true"
label="Provider">
<help><![CDATA[
Request from provider only. If none is selected, query all providers.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

None is not an option, and optional=false

@bienchen bienchen Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I change the text to "If no provider is selected...". Works in planemo serve if I do not select any provider. Then the query includes all providers.

]]></help>
<expand macro="provider_options"/>
</param>
<param name="exclude_provider" type="select" optional="true"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you exclude multiple providers?

Should exclude providers have priority over include_providers? Means you skip them even if the user has choosen then in include_providers ... so that you avoid the error message?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, only single provider. So in case include_provider == exclude_provider the 3D-Beacons API 404s, the error in front of that saves an unnecessary API call. I know the error is a bit awkward and your suggestion will prevent it. So I'm going to implement it but give priority to include_provider because with no provider, there would never be a result.

Comment thread tools/3dbeacons_api/3dbeacons_api.xml Outdated
<expand macro="provider_options"/>
</param>
<param name="template" type="text" value="" optional="true"
label="Template">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Protein Template"?

]]></help>
<validator type="regex" message="Template must be a 4-character PDB ID (e.g., '1ake') optionally followed by assembly and chain (e.g., '1ake.1.A', '1ake1A').">^[a-zA-Z0-9]{4}([._-]?\d+[._-]?[A-Za-z])?$</validator>
</param>
<param name="sequence_range" type="text" value="" optional="true"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can do that, or use two integer params. Two integer params make a better API that can be validated with pydatic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How do I make sure that both, start & end are set, as soon as one value is provided? I don't see how I can check "if start is not None and end is None: ERROR" outside of the command element. <validator> can not see values outside of its own <param> scope, right?

@bgruening

Copy link
Copy Markdown
Member

@bienchen can you please remind me why we can not add a real world test? Your outputs with nested lists look complicated and would really need some real tests.

@bienchen

Copy link
Copy Markdown
Contributor Author

@bienchen can you please remind me why we can not add a real world test? Your outputs with nested lists look complicated and would really need some real tests.

In the SWISS-MODEL. wrapper, we skipped that because the API could be down or change and the test would fail because of that, not because of an error in the tool integration. If you think that testing is more important here than possible API issues, I can add a test. Recommendations what to test? single & batch mode or just batch mode because it has the more complex output?

@bgruening

Copy link
Copy Markdown
Member

Yes, I think testing is important, especially that your outputs do have the correct shape etc ... the collections look complicated and should be tested imho. I hope the API is most time up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage/Discuss

Development

Successfully merging this pull request may close these issues.

2 participants