I don't think it's a good idea to rely on PythonNodeBase.DefaultPythonEngine pointing to version 2, as we should eventually make version 3 the default, and then this code would break. Is there some other constant/enum that we can use that's explicitly about Python 2.

@mmisol If we change the value for PythonNodeBase.DefaultPythonEngine, we should update the code here. Let me add a TODO, this is the only new flag we added, there is no other indication. Also I think this alpha test code will be removed later.

I agree with @mmisol, this should check for 2 directly.

@Dewb @mmisol Do you mean checking the actual python2 string value instead of this property? If that's the case, how about we rename it to not call it default? From the API standpoint, referencing the default still do not give me a clue which engine version it is

I just checked out the branch to take a look at the code. Renaming the DefaultPythonEngine constant to PythonNet2Engine (or something with a 2 in it :) ) would be good to me.
Edit: Copying relevant code for reference

        public static readonly string DefaultPythonEngine = "IronPython2";
        public static readonly string PythonNet3Engine = "CPython3";

        private string engine = DefaultPythonEngine;

        public string Engine
        {
            get { return engine; }
            set
            {
                if (engine != value)
                {
                    engine = value;
                    RaisePropertyChanged("Engine");
                }
            }
        }

Order should not be a problem if they were serialized as strings, but I agree that you lose the flexibility of maybe having other python engines loaded dynamically.

I agree enums would be superior here. What we really would like is a type-checked enum that can be dynamically populated by what assemblies you have loaded (so the enum only contains CPython3 if DSCPython.dll is loaded and DSIronPython.dll is not.)

This sort of fancy, module-spanning dynamic enum has been built in C++ for other products. I'm not sure there's an easy technical solution in C#, but it seems possible. I think this is a good area to investigate further in the tasks to make the engines optional; it doesn't need to gate merging this feature.

It might be a good idea to make the property non-public so we preserve the ability to change the API before the Python 3 features are finalized. The property will have to be serialized to a string in .dyn files regardless, so a move to enums won't have to break graphs.

Ah, I see the source of the confusion now. The DefaultPythonEngine static property was not intended to be "the name of Python 2 engine" it was intended to mean "which of the possible values of this string-faked enumeration should be assigned to nodes that do not have a value of the Engine property at deserialization time." The idea was it would be set once here, and referenced wherever we needed to, and in the future when we make the default 3, we can change it in one place.

Ideally, the NodeModel code wouldn't know about the details of either of these engines. We could create a RegisteredPythonEngines data structure and each engine DLL could call a RegisterPythonEngine method during initialization, providing a serialization name and an entry point.

We should also separate the concept of "the default value of this property on new nodes" from "the value we attach for graphs from 2.6 and earlier." We will want to change the default engine from 2 to 3 in the future, but pre-2.6 graphs should always be set to 2.

these are important point's Mike - thanks for clarifying them.

hmmm I think contractor's work especially the UI part and editor switch will need the whole series of the values somewhere. Do you have a preference between Enums with display name or set of strings? I think we need them available anyway. We can implement the dynamic list later but probably need a static list to solve the immediate needs.