ProcessBuilder: ensure instances do not interfere

[sphuber]

Fixes #4420

There was a bug in the ProcessBuilderNamespace constructor where it
converts the ports of the port namespace into dynamic properties and
assigns them to the class. The problem here is that the property is
added to the class instead of the instance. This meant that if you have
two classes that happen to have ports with overlapping names, when
constructing the builder for the second, the properties of the first
builder that have port names.

The simple solution of simply add the dynamic properties to the instance
instead of the class won't work, because properties are descriptors and
can only be assigned to a class. The workaround is to create a subclass
on the fly and attach the properties to that new class.

[codecov]

Codecov Report

Merging #4984 (d9f4940) into develop (59cee4a) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #4984      +/-   ##
===========================================
- Coverage    80.38%   80.38%   -0.00%     
===========================================
  Files          529      529              
  Lines        36862    36867       +5     
===========================================
+ Hits         29628    29632       +4     
- Misses        7234     7235       +1     

Flag	Coverage Δ
django	74.88% <100.00%> (+0.01%)	⬆️
sqlalchemy	73.79% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aiida/engine/processes/builder.py	92.99% <100.00%> (+0.33%)	⬆️
aiida/transports/plugins/local.py	81.41% <0.00%> (-0.25%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59cee4a...d9f4940. Read the comment docs.

[chrisjsewell]

I take it this is built on (and requires first merging) #4983?

[sphuber]

I take it this is built on (and requires first merging) #4983?

It does indeed. The changes in this PR are minimal and can already be reviewed, but it is not the easiest to distill them from the other changes. Thats why I marked it blocked so we can merge the other first and then this should be straightforward

[chrisjsewell]

ugghh, this doesn't look quite kosher lol. Are sure we cannot use __getattr__ here? (as already mentioned in #4420 (comment))

[sphuber]

ugghh, this does look kosher lol.

I think you mean doesn't ?

Are sure we cannot use __getattr__ here? (as already mentioned in #4420 (comment))

Feel free to try. But with the properties we can get add the help string of the port to the __doc__ of the property. Don't think we can emulate this behavior using __getattr__.

[sphuber]

@chrisjsewell I am merging this tomorrow. So if you still want to have a look at another solution, please do so a.s.a.p.

[chrisjsewell]

So if you still want to have a look at another solution, please do so a.s.a.p.

ok gimme til midday tomorrow

[chrisjsewell]

Hmm, ok I'll approve this.

I get that the dynamic attribute access help in the context of ipython shells, but urgh, it does feel these builders are getting overly complex.
Then again, I don't have a better solution right now 🤷