Issue #6687 Fix QueryBuilder filtering for AbstractCode #6857
Conversation
danielhollas
left a comment
danielhollas
left a comment
There was a problem hiding this comment.
@adityagh006 please push the tests as well.
pyproject.toml
Outdated
| 'core.arithmetic.multiply_add' = 'aiida.workflows.arithmetic.multiply_add:MultiplyAddWorkChain' | ||
|
|
||
| [project.entry-points.'aiida.orm'] | ||
| 'core.code.abstract' = 'aiida.orm.nodes.data.code.abstract:AbstractCode' |
There was a problem hiding this comment.
I think this entry point should be moved to [project.entry-points.'aiida.data'] section where the other code entry points are defined.
src/aiida/orm/querybuilder.py
Outdated
| # value = classifiers.ormclass_type_string | ||
|
|
||
| # if not subclassing: | ||
| # filters = {'==': value} | ||
| # else: | ||
| # # Note: the query_type_string always ends with a dot. This ensures that "like {str}%" matches *only* | ||
| # # the query type string | ||
| # filters = {'like': f'{escape_for_sql_like(get_query_type_from_type_string(value))}%'} | ||
|
|
||
| value = classifiers.ormclass_type_string | ||
|
|
||
| if value == 'data.core.code.abstract': | ||
| value = 'data.core.code' # Ensure it matches all codes | ||
|
|
There was a problem hiding this comment.
| # value = classifiers.ormclass_type_string | |
| # if not subclassing: | |
| # filters = {'==': value} | |
| # else: | |
| # # Note: the query_type_string always ends with a dot. This ensures that "like {str}%" matches *only* | |
| # # the query type string | |
| # filters = {'like': f'{escape_for_sql_like(get_query_type_from_type_string(value))}%'} | |
| value = classifiers.ormclass_type_string | |
| if value == 'data.core.code.abstract': | |
| value = 'data.core.code' # Ensure it matches all codes |
| if not subclassing: | ||
| filters = {'==': value} | ||
| else: | ||
| # Note: the query_type_string always ends with a dot. This ensures that "like {str}%" matches *only* |
There was a problem hiding this comment.
Please keep this comment
adityagh006
force-pushed
the
issue-6687-v2
branch
2 times, most recently
from
e226f59 to
1ca1da8
Compare
|
@agoscinski FYI I think this one would be great to get into 2.7. What is your timeline for release? |
|
@danielhollas well it seems that this does not fix the problem. I can query the |
| """ | ||
| # Set up test environment | ||
|
|
||
| computer = orm.Computer( |
There was a problem hiding this comment.
We should use existing test fixtures here instead of managing manually, e.g. aiida_code or aiida_code_installed.
There was a problem hiding this comment.
I think in this case yes, you can just use them. When I had a go I was not yet sure what the test should do. Sometimes you need a computer with a specific label or so. But in this case the fixture should be usable.
I'll give it a spin later today, thanks! |
|
Okay, I figured out the issue with the original code and cleaned up and expanded the tests. @agoscinski please review the second-to-last commit there with the fix. There is also some subtlety with the |
|
Replaced by #6866 |
|
@adityagh006 thank you for your work on this! 🙏 |
3 participants
This PR fixes an issue in QueryBuilder where filtering for AbstractCode was not working correctly. The update ensures that queries involving AbstractCode return expected results.
Changes Made:
1.Fixed filtering logic in querybuilder.py.
2.Updated pyproject.toml accordingly.
3.Added pytest tests for QueryBuilder functionality, as previously requested.
Let me know if any additional changes are needed.
Closes #6687.