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

QueryBuilder can't find codes when looking for AbstractCode #6687

Open
edan-bainglass opened this issue Jan 9, 2025 · 3 comments
Open

QueryBuilder can't find codes when looking for AbstractCode #6687

edan-bainglass opened this issue Jan 9, 2025 · 3 comments
Labels
good first issue Issues that should be relatively easy to fix also for beginning contributors type/bug

Comments

@edan-bainglass
Copy link
Member

Describe the bug

orm.QueryBuilder().append(orm.AbstractCode) will not find any codes, as the resultant query has {'node_type': {'like': 'data.code.abstract.%'}, whereas codes carry an entry point of data.core.code.%.

Steps to reproduce

Try finding any code with orm.QueryBuilder().append(orm.AbstractCode). Count should always come up null.

Expected behavior

Successfully find codes.

Your environment

  • Linux (aiidalab/full-stack image)
  • Python 3.9.13
  • aiida-core v2.6.3

Additional context

orm.QueryBuilder().append(orm.Code) works, yielding {'node_type': {'like': 'data.core.code.%'}. But then you're using the deprecated orm.Code.

edan-bainglass added a commit to aiidalab/aiidalab-widgets-base that referenced this issue Jan 9, 2025
- Add additional guards for missing code setup input
- Disable quick setup button if missing any requisites
- Apply uniqueness to missing template variables warning
- Make `ResourceSetupBaseWidget` "public"
- Fix code full label uniqueness check

Note that the fix in the query that looks for existing codes is a hack due to a potential bug in aiida-core. See aiidateam/aiida-core#6687.
@unkcpz unkcpz added the good first issue Issues that should be relatively easy to fix also for beginning contributors label Feb 13, 2025
@adityagh006
Copy link

Hey, I’m interested in contributing to this issue. I see that orm.QueryBuilder().append(orm.AbstractCode) isn’t returning results due to the node_type mismatch. Would updating the node_type filter in QueryBuilder to match data.core.code.% be a suitable fix, or is there a reason it was originally set to data.code.abstract.%?

@danielhollas
Copy link
Collaborator

Would updating the node_type filter in QueryBuilder to match data.core.code.% be a suitable fix, or is there a reason it was originally set to data.code.abstract.%?

CC @sphuber in case he remembers the design decisions behind AbstractCode.
FWIW I am not sure if this is a "good first issue" kind of thing.

@sphuber
Copy link
Contributor

sphuber commented Feb 24, 2025

The problem here is that the hierarchy of ORM classes are defined differently in Python compared to how it is represented in the database (which is what the QueryBuilder ultimately uses). The ORM classes in Python obviously use Python's class hierarchy system. So since InstalledCode and PortableCode inherit from AbstractCode, instances of the former will also be instances of the latter. However, the class system does not exist in the database. Instead, the querybuilder uses a proxy based on the node type strings. For example, the Data class has data as type string, and its subclass StructureData has data.structure. To find "subclasses" of Data, the query then includes a filter on the node_type that matches data.%.

So for all of this to work, the node_type strings need to be defined such that they correctly reflect the Python class hierarchy. But the type strings are not determined based on the class hierarchy but rather on the entry point strings. This is mostly for historical reasons. Originally (and I mean a long time ago when there was just one AiiDA repository without any plugin system and all code plugins were living in the same repo), the node type string was based on the module hierarchy. When we introduced the plugin system allowing external packages to define ORM classes (Data plugins mostly) this no longer worked and we based it off the entry point string instead. I have to admit that I am not a 100% sure anymore if we tried to use the actual class hierarchy to derive the node string and that didn't work, or we never considered it. One thing that comes to mind is that the node_type is not only used for capturing class hierarchy in querying, but mostly it is necessary to be able to load the correct Python class when retrieving a node from the database. Imagine you get a row from the DbNode table, how do you know which Python class to return an instance of? That is done using the node_type. It is converted to an entry point string, which is then used to load the corresponding class that is registered at that entry point.

With that all out of the way, we now come to the specific problem of the OP. The AbstractCode is a later addition (I think somewhere around AiiDA v2.2 if memory serves me right). Originally there was the Code class and the various types (installed and portable, called remote and local at the time) where represented by this one class. It turned out that this design led to lots of complications and the remote/local naming was confusing (because a remote, i.e. installed code, could also be installed on the localhost, i.e. "locally") so we decided to refactor. But since we needed to keep backwards compatibility, I could not get rid of the Code class and had to come up with a new base class, which is AbstractCode. But on top of that, I could not reappropriate the code entry point string of Code either. I just checked and noticed that the AbstractCode doesn't actually even seem to be registered as an entry point:

'core.code.containerized' = 'aiida.orm.nodes.data.code.containerized:ContainerizedCode'

This has presumably not yet given rise to any problems because it is never actually instantiated, only its subclasses. But I am a bit suprised as to how the query string is generated then when calling QueryBuilder().append(AbstractCode).

Anyway, I think the solution should be the following:

  1. Register AbstractCode in pyproject.toml as 'core.code.abstract' = 'aiida.orm.nodes.data.code.abstractAbstractCode'
  2. Add a special case/clause in the QueryBuilder code that when AbstractCode is appended, the node_type query string is not data.core.code.abstract% but data.core.code% instead.

I think querying for AbstractCode with subclassing enabled should then match subclasses as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that should be relatively easy to fix also for beginning contributors type/bug
Projects
None yet
Development

No branches or pull requests

5 participants