search

imglib / imglyb

Connecting Java/ImgLib2 + Python/NumPy
https://pypi.org/project/imglyb/
BSD 2-Clause "Simplified" License
31 stars 5 forks source link

OSXAWTwrapper.main() unused app variable #17

Closed gselzer closed 2 years ago

gselzer commented 2 years ago

This line is not PEP8-compliant, as app is an unused variable. But, looking at the documentation, I am hesitant to delete it as it creates an application instance if it does not exist. Can we just delete this? (cc @ctrueden, the author of said line).

Unfortunately, this code is not tested :frowning:

gselzer commented 2 years ago

Fixes should be added to #18, as that PR is blocked by this flaking error.

ctrueden commented 2 years ago

Why not just remove the app = portion, and write NSApplication.sharedApplication() ? Edit: I pushed it.

And yeah, this code cannot be tested on non-Mac platforms.

This code is mostly obsolete anyway, thanks to jpype.setupGuiEnvironment. But I'm hesitant to delete it nonetheless, in case it is useful in the future...

gselzer commented 2 years ago

Why not just remove the app = portion, and write NSApplication.sharedApplication() ? Edit: I pushed it.

Eh, we can, I was hoping that we could either delete it or document it...

And yeah, this code cannot be tested on non-Mac platforms.

We can just write a test that only runs if the platform is Mac? It will always be tested by Github Actions...

Anyways, thank you for making that change!

ctrueden commented 2 years ago

We can just write a test that only runs if the platform is Mac?

If you have time, go for it. But it is not very important. That code is unlikely to change again, and now it passes the linter.