import causing window to open

[metabyte144]

trafficstars

Software Versions

• Python: 3.10
• OS: Windows 10
• Kivy: 2.2.1
• Kivy installation method: pip

Describe the bug A import from kivy.core.window import Window should not open a window. That means that the module init is executing code with massive unexpected side effects. Further this prevents the use of kivy in a GUI process, other than the main process and is a major obstacle for multiprocessing.

Constructor code execution that causes side effects, is a major design problem in all OOP languages and should be considered a bug.

Expected behavior No kivy.core.window module init side effects

To Reproduce A short, runnable example that reproduces the issue with latest kivy master.

Code and Logs and screenshots

from kivy.app import App
from kivy.uix.gridlayout import GridLayout
from kivy.uix.button import Button
from kivy.core.window import Window
from multiprocessing import Process

class DoClose(App):

def build(self):
    self.window = GridLayout()
    self.window.cols = 1
    self.window.size_hint = (0.6, 0.7)
    self.window.pos_hint = {"center_x": 0.5, "center_y": 0.5}
    self.button = Button(text="SHUTDOWN", size_hint=(1, 0.4), bold=True, background_color='#00FFCE')
    self.button.bind(on_press=self.stop)
    self.window.add_widget(self.button)
    return self.window

def gogogo():
DoClose().run()

if name == "main":
proc = Process(name="gcon", target=gogogo)
proc.start()

Additional context This issue has been suggested by @akshayaurora as follow-up to #8338

[Julian-O]

I think you have stumbled over a systemic issue in the Kivy architecture. However, I do not think your characterisation is accurate.

Constructor code execution that causes side effects, is a major design problem in all OOP languages and should be considered a bug.

It isn't that the constructor has side-effects that is the problem. It is that the constructor (or indeed any code) is being run implicitly on import.

Unless it is the top-level code of a main script, Python modules should avoid having side effects when imported (apart from the obvious declaration of identifiers in the namespace).

However, there are several modules in Kivy that have side-effects - even in __init__.py files. As a result, merely importing Kivy modules can break your code in unexpected ways.

I have seen the core Kivy developers defend this practice as being best for the common case. I strongly disagree with them, but I do not see this getting fixed, especially as too much client code now depends on those side-effects.

In the case of logging (where side-effects on import of any Kivy module were breaking my non-Kivy libraries), I contributed an environment variable check to Kivy to turn off the side-effects. A similar approach might be possible if you are willing to contribute PRs, but in the meantime you need to ensure that kivy.core.window (or, indeed, any Kivy module) is not imported by any process except the one that owns the GUI.

[akshayaurora]

@Julian-O I think it might be time to revisit those issues, a import should ideally not be causing such issues, I would suggest opening a issue possible with a pr if you are willing to work on fixing those issues.

Let's keep that separate from this particular issue though.

[Julian-O]

Some history:

• #4744 complains about this - this implicit Window creation was acting differently on different platforms. It was closed after some back and forth attacks/defence of implicit actions on imports.
• #7211 complains about this - the implicit Window creation was precluding subclassing. Was closed when an alternative approach was found.
• #7742 complains about this - seems like an exact duplicate to this issue. Was closed when an user found the workaround solution.
• #8222 complains about this - the implicit Window creation hampered automated unit tests. Remains unsolved.

[For clarity: I would love to see this permanently fixed, and don't believe it ever can be.]

[metabyte144]

Probably I could work on a PR, at the moment I have no kivy system knowledge

[Samael-TLB]

Upon inspection of the issue, I have nabbed down the place that needs change. The other thing that is needed is to identify crucial functionalities or dependencies that need the window to be initialised. What we can do is move the code that's making the window appear to be invoked with a check , we can have an eager optional keyword argument to the window base constructor( init function) which is by default true and would allow window creation as is currently done. If that variable is set to false the window will not be created on initialization. With this, we can have an ensure_window function in the window base class that can be invoked before accessing functionalities that need the window to be initialized. This is simple as we just need to identify those critical places ( which I tend to believe would be very less in number) and then insert the ensure_window function at the very start of that functionality. Moreover, we can have another variable which can signify that the window has been initialised. This variable would allow the guarantee that the window is created just once no matter how many times the ensure_window function is called. In short, this proposal should in theory provide least amount of friction to current projects while also dealing away with the issue at hand. If @akshayaurora and @Julian-O can agree on the proposal, given by me, I can start working on the pr for the same.

[Julian-O]

Just repeating back, to show I understood: You are planning on delaying the side-effect (create a singleton window) until any code that actually needs it runs, rather than doing it on import.

My thoughts:

• Sounds like a sensible plan.
• If the code that needs it is being run frequently (e.g. every event), the extra check might be inefficient. I am sure we can find solutions in that case. I don't want to start optimising too early.
• The reason why I couldn't use this approach with my clean up of logging was: Importing kivy.__init__.py had implicit side-effects on the logging subsystem, and then also used the logging subsystem, so the side-effects had to already be in place! So my question for you is: Do any of the normal imports (and kivy.core.window in particular) need to run the ensure() code? That would make the approach more complex.
• The hard question: Do clients expect the side-effects in place? Are there any operations a client might do outside of the code that will now be protected by ensure() calls that assumes the windows is created? This is my biggest concern - largely because it is an unknown to me.
• You don't need my approval! I am not that important in this project, and my opinions don't necessarily reflect those of management.

[Samael-TLB]

Yes, @Julian-O to highlight my proposal in details, I propose the below changes:

1. We introduce a variable named eager which can be used for deciding whether the window creation process is done eagerly or lazily. By default, its set to True so that no changes occur in any other code that depends on the current behavior. It can be explicitly set to False in config file ( or some other place as per suggestion), which then creates the window lazily at the time of requirement of the window.

2. In the init function of the class WindowBase, we can simply do the following changes: a) Current code:

    # before creating the window
    import kivy.core.gl  # NOQA
   
    # configure the window
    self.create_window()
    self.register()
   
    # manage keyboard(s)
    self.configure_keyboards()
   
    # assign the default context of the widget creation
    if not hasattr(self, '_context'):
        self._context = get_current_context()
   
    # because Window is created as soon as imported, if we bound earlier,
    # metrics would be imported when dp is set during window creation.
    # Instead, don't process dpi changes until everything is set
    self.fbind('dpi', self._reset_metrics_dpi)
   
    # mark as initialized
    self.initialized = True
   

   b) Proposed change:

    eager= Config.get('kivy', 'eager_window_creation')
   
    if eager:
         # before creating the window
         import kivy.core.gl  # NOQA
   
        # configure the window
         self.create_window()
   
    self.register()
   
    # manage keyboard(s)
    self.configure_keyboards()
   
    # assign the default context of the widget creation
    if not hasattr(self, '_context'):
        self._context = get_current_context()
   
    # because Window is created as soon as imported, if we bound earlier,
    # metrics would be imported when dp is set during window creation.
    # Instead, don't process dpi changes until everything is set
    self.fbind('dpi', self._reset_metrics_dpi)
   
    if eager:
        # mark as initialized
        self.initialized = True
   

3. We can have a function (naming to be decided) which encapsulates the two lines:

    def name_to_be_decided(self):
        if  not self.initialized:
            # before creating the window
             import kivy.core.gl  # NOQA
   
            # configure the window
            self.create_window()
            self.initialized=True
   

This will ensure enforcement of creating/importing the gl context before window creation

4. Now, we just need to put this function described in 3rd point above as the first line in functions which require capabilities which need the window to be initialized. Upon my thorough investigation through the codebase, I found that those important places, that need the window to be initialized, are already having a guard statement to ensure the window is present/initialized. This is done through the EventLoop.ensure_window() function. So, we just need to patch this function to work correctly with our latest changes. a) Current code:

          def ensure_window(self):
              '''Ensure that we have a window.
              '''
              import kivy.core.window  # NOQA
             if not self.window:
                  Logger.critical('App: Unable to get a Window, abort.')
                  sys.exit(1)
   

   b) Proposed change;

          def ensure_window(self):
              '''Ensure that we have a window.
              '''
              from kivy.core.window  import Window# NOQA (can may be move it to top to prevent PEP8 error?)
              Window.name_to_be_decided()
             if not self.window:
                  Logger.critical('App: Unable to get a Window, abort.')
                  sys.exit(1)
   

This simple changes will allow us to not have any (visible) side-effects during initialization/import of the Window class, while also ensuring that existing behaviour is not hampered ( which will ensure downstream usage is not broken). This will also make sure that the changes not expose any footgun to the end users.

Meanwhile, during the investigation of the codebase, I found quite a few discrepancies in the coding style and circular imports. Like, the kivy.core.gl module is imported in the kivy.core.window module, while kivy.core.gl also imports kivy.core.window. This as per my understanding can be removed without any issues from the kivy.core.gl. Also, an example for coding style discrepancy is that in some places the Window object is directly used for window related functionalities, whereas in other places EventLoop.window is being used, e.g., in app.py, we have:

def _run_prepare(self):
    if not self.built:
        self.load_config()
        self.load_kv(filename=self.kv_file)
        root = self.build()
        if root:
            self.root = root
    if self.root:
        if not isinstance(self.root, Widget):
            Logger.critical('App.root must be an _instance_ of Widget')
            raise Exception('Invalid instance in App.root')
        from kivy.core.window import Window #(see this line)
        Window.add_widget(self.root)

    # Check if the window is already created
    from kivy.base import EventLoop
    window = EventLoop.window #(see this line)

Also, we have some unused and possibly deprecated code in the kivy codebase too, like the EventLopp.run function, this is used nowhere else and it seems to me that the function has been deprecated in favour of the EventLoop.mainloop function. So, we can definitely have such obvious pruning for better and lighter codebase.

• If the code that needs it is being run frequently (e.g. every event), the extra check might be inefficient. I am sure we can find solutions in that case. I don't want to start optimising too early.

Upon, checking this, I figured that the code that needs this is not run frequently, actually, its not even run for every event. Its used in certain places and mostly during initialization of the classes like in Widget class. So, the efficiency should not be hampered. Also, in the eventloops, the variable initialized is being used as guard clause, which would point to the fact that this addition would lead to minimum efficiency hamper.

• Do any of the normal imports (and kivy.core.window in particular) need to run the ensure() code?

I believe this has been answered by my detailed proposal above.

• Do clients expect the side-effects in place? Are there any operations a client might do outside of the code that will now be protected by ensure() calls that assumes the windows is created?

As my proposal would retain the current behaviour inplace, and would only have changes in the behaviour for advanced users ( or users having specific needs, which may be a small fraction of the total users) , who need to explicitly change the config for lazy initialization of the window, this would only refer to the fact that the users who do lazy initialization might know what they want to achieve. And also with regards to you second question, as the current kivy code as well as some examples in the tests, show the usage of EventLoop.ensure_window, I believe for the current scenario there should not be any issue on this.

• You don't need my approval! I am not that important in this project, and my opinions don't necessarily reflect those of management.

As you and others, who are contributing to the kivy codebase regularly, are may be somewhat familiar with the design philosophy for the codebase, hence I asked to enquire if my proposal is feasible or I might be missing somethings.

Let me know your thoughts on this detailed proposal.

[Julian-O]

My suggestions (which can be ignored - I am not a reviewer):

• I can't see any substantial issues.
• We now have ensure_window() and name_to_be_decided() both doing similar tasks. I wonder if we should merge them into one, in WindowBase.
• You don't show where you initialize initialized to False
• Consider whether a client needs to see initialized. You should probably name it _initialized.
• Consider merging the two part of the code in WindowBase which check eager.
• Consider renaming eager to lazy (and reversing the defaults). I know lazy and eager are antonyms, but I think lazy is the one used more often, so will be easier for people to recognise its role.
• # because Window is created as soon as imported, This comment needs an update.
• I looked briefly at the import issue. Module A has side-effects on import. Module B imports module A, at the end of its declarations, so it triggers Module A's side effects. Module A imports module B for its declarations but only in an init method (I assume to get around the cyclic dependency issues). I quickly closed the window, so I didn't need to see any more.
• I was unhappy to learn that sys.exit() is called, and so I raised #8348 in response. I do not think you should "fix" this in this proposal.

[Samael-TLB]

Sure, I would adhere to your suggestions and possibly try to have a pr being submitted for this soon.

• We now have ensure_window() and name_to_be_decided() both doing similar tasks. I wonder if we should merge them into one, in WindowBase.

Yes that's the goal , but for minimum breakage , I suggest to have the core function in WindowBase and having the ensure_window() call the core function from Window class.

• You don't show where you initialize initialized to False

The variable initialized is already being utilized in the WindowBase class and initially intialized to False in the __init__(). Though there is already a variable __initialized in the WindowBase class, though its not used anywhere. Maybe, the philosophy was same to have an internal variable and an external read only one for the users. But, that isn't the case here currently. So the variable initialized is being used directly and may be overriden by the users and be used to create multiple windows, which defies the guard that uses this variable. May be that may need another pr.

• Consider merging the two part of the code in WindowBase which check eager.

Could do that but as we are only encapsulating the

  import kivy.core.gl  # NOQA

 # configure the window
  self.create_window()

and not these

 self.register()

 # manage keyboard(s)
 self.configure_keyboards()

 # assign the default context of the widget creation
 if not hasattr(self, '_context'):
      self._context = get_current_context()

 # because Window is created as soon as imported, if we bound earlier,
 # metrics would be imported when dp is set during window creation.
 # Instead, don't process dpi changes until everything is set
 self.fbind('dpi', self._reset_metrics_dpi)

and after all these setup initializations then only self.initialized is set to True, which makes sense logically, but upon further investigation I found that these second functions aren't actually dependent on the actual creation of the window. But can function normally without the window being created( without bugs) and can gracefully exit out without any errors which adhere to the current behaviour.

Now, the question is do we keep them uncapsulated by the lazy guard clause to keep same workflow as these are unrelated by implementation to the actual window creation or shall we also encapsulate these in the lazy guard clause which will ensure logically chronological order and should not sprout any other unknown bug which may be the case in the first way.

• I looked briefly at the import issue. Module A has side-effects on import. Module B imports module A, at the end of its declarations, so it triggers Module A's side effects. Module A imports module B for its declarations but only in an init method (I assume to get around the cyclic dependency issues). I quickly closed the window, so I didn't need to see any more.

So what is your(@Julian-O ) suggestion for this actually, shall we remove the call from the kivy.core.gl or not? Can you detail it out please?

• I was unhappy to learn that sys.exit() is called, and so I raised https://github.com/kivy/kivy/issues/8348 in response. I do not think you should "fix" this in this proposal.

Agreed, I believe the prs should be as atomic as possible and solve at max one main issue at once.

Lastly, what should we name the function name_to_be_decided()?

[Julian-O]

• My question about merging the functions is: Will name_to_be_decided() check that the window is created and call sys.exit()? Or will that remain in ensure_window()

If the former, I suggest the function is called create_window_if_needed() and if the latter, I suggest ensure_window().

• Re: Initialized - Got it, thanks.

• Re: Should we protect self.configure_keyboards() and self.register()? I would argue yes. Why go to all this trouble to prevent one side-effect, when we can prevent three.

In fact, looking again, I wonder if the original call to core_select_lib() should be made lazy, rather than the create_window() call.

• Re: "shall we remove the call from the kivy.core.gl or not" I have no idea. That mess scared me. I don't know. Sorry.

[Samael-TLB]

• My question about merging the functions is: Will name_to_be_decided() check that the window is created and call sys.exit()? Or will that remain in ensure_window()

Call to sys.exit() should logically( if at all it should be an sys.exit) be in the EventLoop.ensure_window() as by principle of separation of concerns exiting should be a concern for EventLoop and not WindowBase as by naming a window is just a surface and should not be bothered with exiting system/app.

If the former, I suggest the function is called create_window_if_needed() and if the latter, I suggest ensure_window().

I too had the same namings in mind @Julian-O , with one another which may be more appropriate if we are encapsulating all three, the naming is initialize_window_setup(). The argument is we are not only initializing the window but also the associated setups in the other two function too. Though from the other perspective of calling this function from EventLoop.ensure_window() , ensure_window sounds good or else we can have ensure_window_is_setup or ensure_window_is_initialized but the contention is in the WindowBase class we aren't ensuring window initialization but rather performing initialization of the window, so may be initialize_window_setup() is better.

In fact, looking again, I wonder if the original call to core_select_lib() should be made lazy, rather than the create_window() call.

After looking at the current scenario, it seems the core_select_lib is just concerned with selection of the backend lib and providing an instance of that. This can be easily deferred in core_select_lib by not instantiating the classes but that would lead to instantiation to be done by the user or some other mechanism for all the different backend libs which might be cumbersome. Also, in actuality, we won't be reducing the side-effects but just delaying them. So, in my opinion, it may be better to do that in the base classes of the respective core backends only, as most of the code is designed to work in that way, just like the case in WindowBase. This would also ensure separation of concerns and also minimization of changes needed overall.

[Julian-O]

it seems the core_select_lib is just concerned with selection of the backend lib and providing an instance of that.

The selection of the lib seems reasonable. It is the construction of a WindowBase instance that seems premature, but I bow to your opinion here.

I think this is ready for a PR.

Don't forget the unit-tests and documenting this new parameter in the docstrings and the .rst files.

[Samael-TLB]

Don't forget the unit-tests and documenting this new parameter in the docstrings and the .rst files.

Sure, I will keep your advice in mind and also get a pr submitted for the same soon.

[Julian-O]

#7104 is another example of this causing a problem.

[Samael-TLB]

If it will be right then I can change my pr to address #8348 and #7104 and raise an exception or warning instead of sys exit. I believe as logging is enabled it should then be just having to log an appropriate log and not raise an issue may be as that should justify the usage in a larger complex system.

[Julian-O]

Sorry, @Samael-TLB, it is late here and I can't remember all the details of what your PR does.

#8348 asks for (almost) all of the sys.exit() to be removed. This issue asks for windows not to be create on import.

#7104 has been closed as a duplicate, because it asks for both of those things.

[Samael-TLB]

Yeah I agree may it would be proper to address #8348 in a separate pr and not this one.

[Julian-O]

#6796 is another consequence of opening windows on import.

[ikus060]

I'm working on a new kivy application and I'm running into issue because from kivy.core.window import Window is creating a window on import.

First scenario, This occur when trying to run test on a headless machine. Since pytest is scanning the modules tree to search for test, when this import is encounter, the pytest execution is failing. That make it impossible to run my test on a headless Linux or Window machine.

Second, when trying to run pyinstaller for my project, it's failing because it also scan the module tree to identify dependencies. The side effect is creating a Window. In this particular case, the window cannot be created because it's a VM without hardware acceleration. Only OpenGL 1.1 is available. So my Pyinstaller is not working.

Any workaround available to avoid creating a Window on import !?

[Julian-O]

@ikus060: This is not the place to discuss your code. Please take it to the support channels.