[patch] Use ImportAlarm from pyiron_snippets

[liamhuber]

No description provided.

[liamhuber]

OSX failure:

 ======================================================================
FAIL: test_with_executor_wait (flex.test_pythonfunctioncontainer.TestPythonFunctionContainer.test_with_executor_wait)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/pyiron_base/pyiron_base/tests/flex/test_pythonfunctioncontainer.py", line 86, in test_with_executor_wait
    self.assertTrue(job.server.future.done())
AssertionError: False is not true

----------------------------------------------------------------------

[liamhuber]

I took a look at the test file and I don't see anything related to ImportAlarm there; I believe this is unrelated. I'm going to try simply re-running the test.

[liamhuber]

I took a look at the test file and I don't see anything related to ImportAlarm there; I believe this is unrelated. I'm going to try simply re-running the test.

Yeah, that did the trick. @jan-janssen I guess there is some stochastic failure in flex.test_pythonfunctioncontainer.TestPythonFunctionContainer.test_with_executor_wait

[jan-janssen]

Why was this removed?

[liamhuber]

Because I wanted to make sure this PR passes unittests. With the branch specified like that, unit tests don't run on stacked PRs.

[liamhuber]

It's in the commit message: Run unittests on stacked PRs too

[jan-janssen]

I find this confusing - should not we do the major change and just enforce the switch to using the ImportAlarm only from pyiron_snippets?

[liamhuber]

I propose to leave this PR with the slightly-confusing-but-backwards-compatible redirection here, but I stacked an additional, compatibility breaking patch in #1458.

I agree that this might be slightly confusing, but I don't think it's explicitly wrong. So what I'd like to do is get go through and approve each part of the stack individually, then we can merge the whole thing in at once. The mild confusingness will thus still exist at a particular commit, but not be included in any releases.

[liamhuber]

I've converted this PR to a draft until we settle on a solution for the compatibility.

[jan-janssen]

From my perspective a major bump and fully removing the ImportAlarm from base would be the preferred solution. Or alternative if we want to be graceful we could replace the ImportAlarm class in pyiron_base with an error message to notify the user they have to upgrade.

[liamhuber]

From my perspective a major bump and fully removing the ImportAlarm from base would be the preferred solution. Or alternative if we want to be graceful we could replace the ImportAlarm class in pyiron_base with an error message to notify the user they have to upgrade.

I'm completely comfortable with both these solutions -- @pmrv do you have a strong opinion here?

If we're going to bump the minor version for this though, let's wait and stack in the rest of the stuff that can come from snippets (Deprecator, deprecate, and deprecate_soon), and get downstream repos already depending on the snippets source (started already here)

[liamhuber]

PRs before ImportAlarm can be removed from the API:

• [patch] use ImportAlarm from pyiron_snippets pyiron_atomistics#1431
• [patch] use ImportAlarm from pyiron_snippets pyiron_gpl#193
• [patch] use ImportAlarm from pyiron_snippets pyiron_continuum#317
• [patch] use ImportAlarm from pyiron_snippets pyiron_contrib#1066

[liamhuber]

PRs before the deprecate stuff can be removed from the API

• [patch] use deprecate from pyiron_snippets pyiron_atomistics#1432
• [patch] use deprecate from pyiron_snippets pyiron_contrib#1067

[jan-janssen]

PRs before the deprecate stuff can be removed from the API

• [patch] use deprecate from pyiron_snippets pyiron_atomistics#1432

pyiron_atomistics is save because it has an upper bound on pyiron_base. So we are just waiting for pyiron_contrib. I set this pull request to draft until things are cleaned up in pyiron_contrib.

[liamhuber]

PRs before the deprecate stuff can be removed from the API

• [patch] use deprecate from pyiron_snippets pyiron_atomistics#1432

pyiron_atomistics is save because it has an upper bound on pyiron_base. So we are just waiting for pyiron_contrib. I set this pull request to draft until things are cleaned up in pyiron_contrib.

My comment is anyhow outdated; the changes to the API don't occur until #1458, the comments were just made before that PR existed and I thought those changes might get made here. So this PR doesn't need to be draft anymore.

[jan-janssen]

Looks good to me