Fix iterjobs() without database

[jan-janssen]

No description provided.

[jan-janssen]

@liamhuber It would be great if you can take another look at this pull request. It took far too long on my side but I definitely want to merge it soon, to move on.

[liamhuber]

I appreciate the comment above the magic-value list, I think that clarifies it perfectly.

Taking a deeper dive, there are a few things I still don't like about this PR:

1. It is made to patch a bug, but there is no change to the test suite. I can't tell it's fixing the bug without manually running the example in iter_jobs() fails without database  #1221, and without a test we're not guaranteeing that this new and improved behaviour will stick around.

2. The high-level inspect and load calls get replaced with the low-level load_from_jobpath call. It didn't take much digging to see why -- load and inspect don't accept db_entry, but these high-level calls were created for a reason: to more clearly signify the intent of the action. Instead, let's empower them to also handle a db_entry instead of a job specifier.

3. By forcing convert_to_object=True in line 619, we are changing behaviour. This actually invalidates the signature of iter_jobs as convert_to_object becomes totally disused. I see the comment a couple lines above that we will deprecate convert_to_object...but deprecate it to what? Always false or always true? Now we're deprecating the variable, but the comment sticks around?? I don't know what the intended future behaviour is, but without clarifying that and updating the "backwards compatibility" comment, I'm not comfortable changing the behaviour.

I'm stacking a patching PR now that addresses all three of these concerns. EDIT: #1228

[liamhuber]

Nit: it is a bit of a smell to if not isinstance(); else instead of if isinstance(); else as it increases cognitive load to mentally negate, but I also see the argument for putting the "default" case at the top so I want to point it out but am comfortable either way.

[jan-janssen]

1. It is made to patch a bug, but there is no change to the test suite. I can't tell it's fixing the bug without manually running the example in iter_jobs() fails without database  #1221, and without a test we're not guaranteeing that this new and improved behaviour will stick around.

I agree - thanks for adding the test.

2. The high-level inspect and load calls get replaced with the low-level load_from_jobpath call. It didn't take much digging to see why -- load and inspect don't accept db_entry, but these high-level calls were created for a reason: to more clearly signify the intent of the action. Instead, let's empower them to also handle a db_entry instead of a job specifier.

From my perspective there are two levels of APIs in pyiron, the user interface which we use interactively and the internal interface the different pyiron objects use to interact with each other. I do not expect a user to write the db_entry in a dictionary to load an object, but for an internal function this is usually easy. Consequently, I do not think the internal calls should be forced to use the same interface as the users.

3. By forcing convert_to_object=True in line 619, we are changing behaviour. This actually invalidates the signature of iter_jobs as convert_to_object becomes totally disused. I see the comment a couple lines above that we will deprecate convert_to_object...but deprecate it to what? Always false or always true? Now we're deprecating the variable, but the comment sticks around?? I don't know what the intended future behaviour is, but without clarifying that and updating the "backwards compatibility" comment, I'm not comfortable changing the behaviour.

This was a bug - that is fixed now.