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

add pgxRpi tool #6805

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

add pgxRpi tool #6805

wants to merge 8 commits into from

Conversation

hangjiaz
Copy link

FOR CONTRIBUTOR:

  • I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • 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)

This PR adds a Galaxy wrapper for pgxRpi, a package that provides a streamlined interface to the Progenetix Beacon v2 API to facilitate efficient and flexible retrieval of genomic variation data.

khaled196

This comment was marked as duplicate.

Copy link
Contributor

@khaled196 khaled196 left a comment

Choose a reason for hiding this comment

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

lowercase

@@ -0,0 +1,80 @@
<tool id="pgx_freqplot" name="pgxRpi pgxFreqplot" version="0.1.0+galaxy0" profile="21.05">
Copy link
Member

Choose a reason for hiding this comment

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

should the version 1.20 and maybe a macro?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for pointing this out. I have included it in a macro.

<option value="hg38" selected="true">hg38</option>
<option value="hg19">hg19</option>
</param>
<param name='plotwidth' type='integer' value='8' label='Width of the plot in inches.' />
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a proper metric system here ;) I propose cm or mm ;)

Copy link
Author

Choose a reason for hiding this comment

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

I have changed it to cm.

<help><![CDATA[
Thie function plots the CNV frequency loaded from pgxRpi
]]></help>
<citations>
Copy link
Member

Choose a reason for hiding this comment

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

maybe put this into a macro?

Copy link
Author

Choose a reason for hiding this comment

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

I have placed the citation in a macro. However, since the help text varies across different functions, I have not included it in a macro.

</param>
<param name='limit' type='integer' value='0' optional='true' label='Integer to specify the number of returned profiles' />
<param name='skip' type='integer' value='0' optional='true' label='Integer specifying the number of profiles to skip' />
<param name='dataset' type='text' optional='true' label='Dataset to query from the Beacon response' help='Use commas to separate multiple datasets to enter' />
Copy link
Member

Choose a reason for hiding this comment

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

How should such a text look like? Is it really a dataset?

Copy link
Author

Choose a reason for hiding this comment

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

This parameter is used to select specific datasets for Progenetix. Currently, Progenetix returns data using only one dataset ID, "progenetix." I have updated the default value of this parameter to "progenetix."

The reason for keeping this parameter is that the package can also query data from other resources via the Beacon API, where "dataset" is a standard query parameter. This allows for more flexible queries across different resources and supports future expansions of Progenetix.

<data name="pgxdata" format="txt" label="${tool.name} for $type data" />
</outputs>
<tests>
<test>
Copy link
Member

Choose a reason for hiding this comment

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

please add tests with more options, especially from the advanced settings

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your suggestions! I have included several tests covering different input types and advanced settings.

However, a few input types, such as “sample_counts,” are not included because these queries return aggregated results. Since Progenetix data is continuously expanding, the results may change over time, causing test failures despite the tool functioning correctly.

Similarly, some parameters, such as “filters,” are not included because their queries return multiple records in a non-fixed order, which could lead to test failures.

<option value="sample_count">count of samples in Progenetix</option>
<option value="filtering_terms">All available filters in Progenetix</option>
</param>
<param name='biosample_id' type='text' optional='true' label='Identifiers of biosamples' help='Use commas to separate multiple IDs (e.g. pgxbs-m3io46hq,pgxbs-m3io41c2). If the output data type is "genomic variations", only this search condition is supported.' />
Copy link
Member

Choose a reason for hiding this comment

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

it would be nice to add some validators to all those text inputs

Copy link
Author

Choose a reason for hiding this comment

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

Please refer to my response above

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.

3 participants