GenericJob: compare to class string rather than static string

[jan-janssen]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.77%. Comparing base (ae94217) to head (30240fa).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1907      +/-   ##
==========================================
+ Coverage   79.75%   79.77%   +0.01%     
==========================================
  Files          79       79              
  Lines       10422    10422              
==========================================
+ Hits         8312     8314       +2     
+ Misses       2110     2108       -2     

☔ 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.

[pmrv]

Is this not now equivalent to isinstance(item, JobCore)? and doesn't this defeat the purpose of using static_isinstance in the first place? I thought the point had been to avoid importing the classes through out the code base.

[jan-janssen]

@pmrv the difference is that static_isinstance() compares the class equality while isinstance() is also true for derived classes, is this case for example GenericJob is isinstance() of JobCore while static_isinstance() would return false. In addition, JobCore is already imported in the file. Finally, the second reason to use static_isinstance() in other cases is to prevent circular imports.

[pmrv]

static_isinstance also traverses the MRO to check whether the first argument is a subclass of the given static string and it's necessary to do this because in this code path the item is a JobPath (that is a subclass of JobCore). What I mean is that we can here just use isinstance directly, since as you said JobCore is already imported anyway and the normal considerations why we have static_isinstance do not apply.

[jan-janssen]

I would disagree, if the object is of type Jobcore then no reload is necessary, if it instead is from a derived type like GenericJob or classes derived from GenericJob then it is necessary that the job object is reloaded.

[pmrv]

I don't disagree on the intended functionality, but you can quickly check that static_isinstance(GenericJob, 'pyiron_base.jobs.job.base.JobCore') == True, so it really is exactly the same as isinstance(GenericJob, JobCore).
I actually think the line (already before this PR) has a bug, as it compares the class of project with JobCore, which is always false, leading to all objects being reloaded after a copy here.
It does not really matter to me which resolution to take here, if you want to stick with the static check, at least it has to be

obj_type=JobCore.__module__ + '.' + JobCore.__name__

because str() adds the funny <class ...> stuff.