Dynamic options: add data table filter #12941

bernt-matthias · 2021-11-17T20:22:45Z

In a recent gitter discussion (https://gitter.im/galaxy-iuc/iuc?at=6190aeff8c019f0d0b93414b) it was asked for a way to have a select list containing only elements that are not yet in a data table.. Et voila .. there is a data table filter.

The test tool shows how it might be used:

the options are dynamically create from a file (from_file also had no test so far)
the filter removes all options that are already in the data table

For the first part I needed to apply a little hack: from_file seems to assume that the file is in tool-data in the Galaxy root. It would be much nicer if the file would be loaded from the dir (or a tool-data dir in this dir) containing the tool. Otherwise we would not meet the use case of the gitter discussion.

I did not think about other possible use cases (e.g. intersections of data tables) and possibly needed attributes...

Ping @mvdbeek @pvanheus

The PR also adds more linting to filters.

How to test the changes?

(Select all options that apply)

I've included appropriate automated tests.
This is a refactoring of components with existing test coverage.
Instructions for manual testing are as follows:
1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

I agree to license these contributions under Galaxy's current license.
I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to [email protected].

mvdbeek

This is really cool, but from_file should not be able to reference files outside of the tool dir.

lib/galaxy/tools/parameters/dynamic_options.py

mvdbeek · 2021-11-18T10:58:27Z

test/functional/tools/filter_data_table.xml

+    <inputs>
+        <param name="dynamic_select" type="select">
+            <!-- initialize options from file (contains all entries that could be added to the data table) -->
+            <options from_file="../test/functional/tool-data/test_file.tsv">


That's a security issue and should not work.

Do you have an idea how to specify another dir during tests, eg test/functional/tool-data/?

I guess we can check if os.path.abspath(path) startswith the tool-data dir... could do this in another PR.

It's fine to do this in another PR, but we shouldn't merge this before we sorted that out.

Also there is a safe_contains function in galaxy.util you can use

ae4c401 contains a possible solution for:

Do you have an idea how to specify another dir during tests, eg test/functional/tool-data/?

Any comments?

mvdbeek · 2021-12-21T09:19:09Z

lib/galaxy/tools/parameters/dynamic_options.py

-                if not os.path.isabs(data_file):
-                    full_path = os.path.join(self.tool_param.tool.app.config.tool_data_path, data_file)
+                full_path = os.path.join(self.tool_param.tool.app.config.tool_data_path, data_file)
+                full_path = os.path.normpath(full_path)


Is the normpath call necessary here ? safe_contains should be smart enough to figure this out, and joining with cwd for non-absolute paths does not seem right (that seems to be what normpath does internally).

mvdbeek · 2021-12-21T09:20:33Z

lib/tool_shed/test/base/twilltestcase.py

@@ -60,7 +60,7 @@ def setUp(self):
        self.galaxy_url = f"http://{self.galaxy_host}:{self.galaxy_port}"
        self.shed_tool_data_table_conf = os.environ.get('TOOL_SHED_TEST_TOOL_DATA_TABLE_CONF')
        self.file_dir = os.environ.get('TOOL_SHED_TEST_FILE_DIR', None)
-        self.tool_data_path = os.environ.get('GALAXY_TEST_TOOL_DATA_PATH')
+        self.tool_data_path = os.environ.get('GALAXY_TEST_TOOL_DATA_PATH', 'test/functional/tool-data/')


If that's necessary for tests to pass I think it likely alerts to the fact that this would break under other circumstances as well.

Found no other way to get the functional test working. But you are right, since there are probably other tests using data tables it makes one wonder why they are working.

I remember that the problem was that GALAXY_ROOT/tool-data was used if I did not change this.

Maybe this is related to the other comment?

Any suggestions how to continue. I'm happy to try anything :)

mvdbeek · 2022-01-26T11:50:55Z

I'm bumping this to 22.05, we still need to figure out what's up with #12941 (comment)

fix missing import

mvdbeek · 2023-06-21T18:31:09Z

There's a conflict here and I think the framework test failure is legitimate ?

test/unit/tool_util/test_tool_linters.py

github-actions bot added area/testing area/tool-framework labels Nov 17, 2021

github-actions bot added this to the 22.01 milestone Nov 17, 2021

bernt-matthias force-pushed the topic/data-table-filter branch from f99c729 to 725f3c2 Compare November 17, 2021 21:19

bernt-matthias changed the title ~~add data table filter~~ dynamic options: add data table filter Nov 17, 2021

bernt-matthias force-pushed the topic/data-table-filter branch from 725f3c2 to d608ba1 Compare November 18, 2021 09:05

mvdbeek reviewed Nov 18, 2021

View reviewed changes

mvdbeek reviewed Dec 21, 2021

View reviewed changes

bernt-matthias force-pushed the topic/data-table-filter branch from 8065a08 to 2a47e13 Compare December 21, 2021 10:46

mvdbeek modified the milestones: 22.01, 22.05 Jan 26, 2022

mvdbeek modified the milestones: 22.05, 22.09 May 25, 2022

dannon modified the milestones: 23.0, 23.1 Dec 8, 2022

bernt-matthias added 6 commits June 6, 2023 10:12

add data table filter

e66f4bb

add more linting for filters

26043cd

use test/functional/tool-data/ as tool-data dir in tests

2470e46

forbid from_file to reference files outside of tool-data

d7c99a0

fix linter and syntax errors

64831e0

add link to tool-data/shared in test tool-data

5b7df0f

fix missing import

bernt-matthias force-pushed the topic/data-table-filter branch from 2a47e13 to bce9300 Compare June 6, 2023 08:28

bernt-matthias added 4 commits June 6, 2023 10:29

remove debug messages

bce9300

fix rebase mistake

95bbd88

one more rebase error

8633181

add context to new linter messages

d199a27

mvdbeek removed this from the 23.1 milestone Jun 21, 2023

mvdbeek marked this pull request as draft June 21, 2023 18:30

bernt-matthias added 4 commits February 16, 2024 15:32

Merge branch 'dev' into topic/data-table-filter

f6d9ca9

add missing import

38e60d6

update black formatting

c1bb7b5

Merge branch 'dev' into topic/data-table-filter

840ebe0

bernt-matthias force-pushed the topic/data-table-filter branch 2 times, most recently from b973bd5 to 50ed0fe Compare February 17, 2024 15:11

update linters and tests to new style

c1091c1

bernt-matthias force-pushed the topic/data-table-filter branch from 50ed0fe to c1091c1 Compare February 17, 2024 15:16

bernt-matthias marked this pull request as ready for review February 17, 2024 15:32

bernt-matthias added this to the 24.0 milestone Feb 17, 2024

mvdbeek modified the milestones: 24.0, 24.1 Mar 5, 2024

mvdbeek modified the milestones: 24.1, 24.2 May 14, 2024

jdavcs changed the title ~~dynamic options: add data table filter~~ Dynamic options: add data table filter Jun 13, 2024

bernt-matthias added 2 commits October 22, 2024 13:23

Merge branch 'dev' into topic/data-table-filter

967db5b

remove unnecessary list comprehensions

e8a612b

bernt-matthias force-pushed the topic/data-table-filter branch from b65deda to e8a612b Compare October 22, 2024 11:35

bernt-matthias added 2 commits October 22, 2024 13:48

fix number of tests

a664638

Merge branch 'dev' into topic/data-table-filter

536f2cf

bernt-matthias commented Nov 6, 2024

View reviewed changes

test/unit/tool_util/test_tool_linters.py Outdated Show resolved Hide resolved

Update test/unit/tool_util/test_tool_linters.py

cde25ed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic options: add data table filter #12941

Dynamic options: add data table filter #12941

bernt-matthias commented Nov 17, 2021 •

edited

Loading

mvdbeek left a comment

mvdbeek Nov 18, 2021

bernt-matthias Nov 18, 2021

mvdbeek Nov 18, 2021

mvdbeek Nov 18, 2021

bernt-matthias Nov 18, 2021 •

edited

Loading

mvdbeek Dec 21, 2021

mvdbeek Dec 21, 2021

bernt-matthias Dec 21, 2021

mvdbeek commented Jan 26, 2022

mvdbeek commented Jun 21, 2023

Dynamic options: add data table filter #12941

Are you sure you want to change the base?

Dynamic options: add data table filter #12941

Conversation

bernt-matthias commented Nov 17, 2021 • edited Loading

How to test the changes?

License

mvdbeek left a comment

Choose a reason for hiding this comment

mvdbeek Nov 18, 2021

Choose a reason for hiding this comment

bernt-matthias Nov 18, 2021

Choose a reason for hiding this comment

mvdbeek Nov 18, 2021

Choose a reason for hiding this comment

mvdbeek Nov 18, 2021

Choose a reason for hiding this comment

bernt-matthias Nov 18, 2021 • edited Loading

Choose a reason for hiding this comment

mvdbeek Dec 21, 2021

Choose a reason for hiding this comment

mvdbeek Dec 21, 2021

Choose a reason for hiding this comment

bernt-matthias Dec 21, 2021

Choose a reason for hiding this comment

mvdbeek commented Jan 26, 2022

mvdbeek commented Jun 21, 2023

bernt-matthias commented Nov 17, 2021 •

edited

Loading

bernt-matthias Nov 18, 2021 •

edited

Loading