/black

/integration-test

just for a discussion, will MONAI have better (backward) compatibility support if we have pytorch-ignite>=0.4.4 here?

Hi @wyli ,

Basically, if we don't have a special dependency on ignite v0.4.5, >=0.4.4 would be better. But I would strongly suggest to use ==0.4.5 for 2 reasons:

1. As we are working on the decollate feature and only the metrics of ignite 0.4.5 can support list of tensors. ==0.4.5 can avoid user's potential issues related to metrics computation.
2. We have some custom logic based on ignite Engine, so we can't guarantee future versions of ignite to be compatible, so should not use >=0.4.4.

When ignite v0.4.6 or v0.5 release, If we don't have any special dependency, we can use pytorch-ignite>=0.4.5,<=0.5.
What do you think?

Thanks.

my main concern is that essentially for any user's code such as optional_import("ignite.engine", "0.4.4", exact_version, "Engine") will also need an update after this PR

is it possible to have pytorch-ignite>=0.4.4 in the requirements, but different optional_import version requirements in the python modules. some of the modules don't strictly require 0.4.5

Hi @wyli ,

Thanks for your suggestion.
Actually, all our core code doesn't depend on ignite v0.4.5, only the examples, integration tests, clara MMARs(future version) need v0.4.5 to compute metrics on a list of tensors.
So I think maybe we can do it in this way:
Use ignite==0.4.5 only in requrement-dev.txt, all the other places or optional_imports use ignite>=0.4.4.
But we need a way to clearly let users know that if want to use ignite metrics with MONAI v0.6, they must install ignite 0.4.5, otherwise, ignite 0.4.4 is good enough.
What do you think?
@ericspod @rijobro what do you guys think about the version of ignite?

Thanks.

@Nic-Ma I have a question if it could be possible to handle package version in 1 or 2 places instead of modifying a lot of python files ? Otherwise, I think >=0.4.4 can work as well

Hi @vfdev-5 ,

Thanks for your review and suggestions.
ignite 0.4.5 is only required in our integration tests and examples & tutorials.

Thanks.

I think @vfdev-5 's point was there there's 28 files being modified here to upgrade the version, most of these are changing the optional_import statements. Perhaps we should concentrate these imports to one file somewhere, if we had an optional module that was imported only a few times optional_import used in place would be fine but now there's a lot of these for Ignite.

Also could we not just require version 0.4.4 as a minimum requirement, eg. Events, _ = optional_import("ignite.engine", "0.4.4", min_version, "Events")?

@ericspod thanks for explaining my point. I was thinking basically about something like:

from monai.utils import exact_version, optional_import, optional_ignite_version

Events, _ = optional_import("ignite.engine", optional_ignite_version, exact_version, "Events")

and such that we could either define optional_ignite_version as a str value (e.g. "0.4.5") or read from setup.cfg ...

/black

Hi @wyli , @ericspod and @vfdev-5 ,

Thanks for your review and suggestions.
Now I changed to use min_version, 0.4.4 in the optional imports but install script still uses 0.4.5 to let users and our CI to use the new metrics feature of ignite 0.4.5.
Do we need to test both ignite v0.4.4 and v0.4.5 to make sure we are OK with >=0.4.4? Maybe we can consider it later.
What do you think?

Thanks.

/black

/integration-test