Skip to content

Replace the dict-based way of declaring sockets and namespaces with the spec API#35

Merged
superstar54 merged 12 commits intomainfrom
use_spec
Aug 15, 2025
Merged

Replace the dict-based way of declaring sockets and namespaces with the spec API#35
superstar54 merged 12 commits intomainfrom
use_spec

Conversation

@superstar54
Copy link
Member

@superstar54 superstar54 commented Aug 13, 2025

This PR replaces the old dict-based way of declaring sockets and namespaces with the spec API from node-graph.

In this way, one can declare the

  • top-level dynamic outputs, output_sepc=spec.dynamic(any)
  • dynamic of namespace, output_spec=spec.dynamic(spec.namespace(sum=int))

Before

@pyfunction(outputs=[{"name": "sum"}, {"name": "diff"}])
def add(x, y):
    return {"sum": x + y, "diff": x - y}

After

@pyfunction(outputs=spec.namespace(sum=any, diff=any))
def add(x, y):
    return {"sum": x + y, "diff": x - y}

Or, one can use type annotation directly:

@pyfunction()
def add(x, y) -> spec.namespace(sum=any, diff=any):
    return {"sum": x + y, "diff": x - y}

@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2025

Codecov Report

❌ Patch coverage is 92.14660% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.15%. Comparing base (a70e9a0) to head (a89a8a7).

Files with missing lines Patch % Lines
src/aiida_pythonjob/utils.py 89.88% 9 Missing ⚠️
src/aiida_pythonjob/ports_adapter.py 94.02% 4 Missing ⚠️
src/aiida_pythonjob/calculations/pyfunction.py 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
+ Coverage   83.72%   84.15%   +0.42%     
==========================================
  Files          19       20       +1     
  Lines        1063     1136      +73     
==========================================
+ Hits          890      956      +66     
- Misses        173      180       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

######################################################################
# Default outputs
# --------------
# -----------------
Copy link
Member

Choose a reason for hiding this comment

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

Just for future reference, it only needs to be as long as the title (less raises an issue, more is unnecessary) 🙂

import cloudpickle

if self._func is None:
if not getattr(self, "_func", None):
Copy link
Member

@edan-bainglass edan-bainglass Aug 15, 2025

Choose a reason for hiding this comment

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

Because ._func is no longer guaranteed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I encountered _func does not exist one time, but I can not reproduce, maybe when reloading the process (e.g., when the daemon restarts). But this should be fixed in another method. I have update it in this commit

Copy link
Member

Choose a reason for hiding this comment

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

Can you explain this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the spec API and its related utilities are defined in node-graph and reused by WorkGraph and PythonJob, which keeps this API as the single source of truth. To avoid exposing the "socket" concept to PythonJob users (since it's an AiiDA plugin that can be used independently of WorkGraph), a simple adapter is used to convert "socket" to "port".
It's hard to decide where we put the spec API, we keep it in the node-graph for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Socket and Port are very similar schemas, but Socket has more metadata (e.g., links).

Copy link
Member

Choose a reason for hiding this comment

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

Okay. Thanks 🙏

Copy link
Member

@edan-bainglass edan-bainglass left a comment

Choose a reason for hiding this comment

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

Got a few questions

Copy link
Member

@edan-bainglass edan-bainglass left a comment

Choose a reason for hiding this comment

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

LGTM!

@superstar54 superstar54 merged commit 30383c6 into main Aug 15, 2025
5 checks passed
@superstar54 superstar54 deleted the use_spec branch August 15, 2025 08:47
@superstar54 superstar54 changed the title Use spec Replace the dict-based way of declaring sockets and namespaces with the spec API Aug 15, 2025
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.

Define the root inputs/outputs as dynamic

3 participants