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 support for Process classes to QueryBuilder.append #2400

Closed
sphuber opened this issue Jan 18, 2019 · 2 comments
Closed

Add support for Process classes to QueryBuilder.append #2400

sphuber opened this issue Jan 18, 2019 · 2 comments
Assignees
Labels
priority/critical-blocking must be resolved before next release topic/orm topic/query-builder type/accepted feature approved feature request
Milestone

Comments

@sphuber
Copy link
Contributor

sphuber commented Jan 18, 2019

If I have the CalcJob class PwCalculation, which has entry point aiida.calculations:quantumespresso.pw, calling QueryBuilder.append(PwCalculation).all() should tell the query builder to return all nodes that have a type string node.process.calculation.calcjob.CalcJobNode. and a process_type string aiida.calculations:quantumespresso.pw. This is essentially an AND statement of these two values. Note that since append supports adding tuples of classes, e.g. QueryBuilder.append((PwCalculation, ArithmeticAddCalculation)), this should be unwrapped into two sub filters that are OR'ed, i.e. in pseudo SQL

        WHERE (
            type = 'node.process.calculation.calcjob.CalcJobNode.' AND
            process_type = 'aiida.calculations:quantumespresso.pw'
        ) OR (
             type = 'node.process.calculation.calcjob.CalcJobNode.' AND
             process_type = 'aiida.calculations:arithmetic.add'
        )

technically since the process_type should also support querying for sub classes, the filter on process type should be an OR of process_type = 'aiida.calculations:quantumespresso.pw' and process_type like 'aiida.calculations:quantumespresso.pw.'

@sphuber sphuber added priority/critical-blocking must be resolved before next release topic/query-builder type/accepted feature approved feature request topic/orm labels Jan 18, 2019
@sphuber sphuber added this to the v1.0.0b1 milestone Jan 18, 2019
@ltalirz
Copy link
Member

ltalirz commented Jan 21, 2019

Just to summarize the logic here:

  • parameter cls: need to provide a class:

    • if an instance is provided (instead of a class), raise an exception
    • if Process class is provided, get corresponding Node class and filter by its type (as well as the process_type)
    • if Node class is provided, just use its type for filtering.
    • if a tuple of classes is provided, use combination logic mentioned in the issue description
  • parameter type: we would need to add an additional parameter process_type. At this point, the question arises whether we really need to have both type and process_type as parameters. Should they just be replaced by filters instead?
    @lekah mentions that type was added just for the REST API. This would indicate that it can simply be replaced by corresponding filters

  • parameter subclassing: if True, include subclasses both of type and process_type (if used to filter).

@sphuber
Copy link
Contributor Author

sphuber commented Mar 1, 2019

Fixed in PR #2421

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/critical-blocking must be resolved before next release topic/orm topic/query-builder type/accepted feature approved feature request
Projects
None yet
Development

No branches or pull requests

3 participants