Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR: Only build Full macOS app on pull requests #18120

Merged
merged 3 commits into from
Jun 15, 2022

Conversation

mrclary
Copy link
Contributor

@mrclary mrclary commented Jun 2, 2022

Description of Changes

Since artifacts for PRs are primarily used by developers for verification, only the Lite version of the macOS app is built on pull requests. On releases, both Lite and Full versions are built. This may free up some CI resources.

Additionally, pandas (and numpy) are added to the Lite build in order for pandas objects to be inspected in the Variable Explorer in the Lite app version.

@spyder-ide/core-developers, let me know what you think of these ideas.

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @mrclary

@dalthviz
Copy link
Member

dalthviz commented Jun 3, 2022

Thinking about it, having pandas and numpy in the lite installer could be nice (at the end if you don't have those bundled the Variable Explorer is quite limited). Also, leaving only one installer build on the MacOS case could be useful since the MacOS slots are quite limited (currently we can only run 5 concurrent jobs I think). However, for testing purposes, I think that leaving the Full version is better since probably with that we can catch more things not working since it has more dependencies involved.

@mrclary
Copy link
Contributor Author

mrclary commented Jun 3, 2022

However, for testing purposes, I think that leaving the Full version is better since probably with that we can catch more things not working since it has more dependencies involved.

This is a good point. @ccordoba12 had previously mentioned that he primarily downloads the Lite version for checking because it is much smaller. I suppose, then, it depends on what most developers use. Can we see stats on how many times the Lite vs. Full version artifacts have been downloaded (not release versions)?

I've done both, but I also lean toward Lite when downloading; but I can easily build the full version locally if I need to test that. Of course, others may not have that preference.

@ccordoba12
Copy link
Member

This is a good point. @ccordoba12 had previously mentioned that he primarily downloads the Lite version for checking because it is much smaller.

That's correct, but only if I need to test completion and linting stuff. For the rest I use the full installer.

Thinking about it, having pandas and numpy in the lite installer could be nice

I agree with @dalthviz on this one because that way we'd have a wider range of possibilities to test our installers. If we plan to leave a single installers build for PRs I think we should leave the full one.

I suppose, then, it depends on what most developers use. Can we see stats on how many times the Lite vs. Full version artifacts have been downloaded (not release versions)?

No idea if Github provides stats for that.

@CAM-Gerlach
Copy link
Member

Just for reference, per my testing with conda with Python 3.8 on a fresh conda env with a fairly clean-ish cache, the solve with just Spyder's base deps, python-lsp-server and jellyfish installs 193 packages for a total of 212 MB downloaded. Adding Numpy and Pandas increases that to 201 packages for a total of 230 MB downloaded, whereas a full install with Cython, SciPy, Sympy and Matplotlib is 234 packages for a total of 327 MB downloaded. Of course, these are approximate numbers as the installer packages won't correspond exactly to the conda ones, but that gives some rough idea.

But yeah, without Numpy and Pandas, Spyder's variable explorer is much more limited. I'm not sure how much that impacts dev installer releases, though, since I only rarely use them for testing (I mostly use a checkout of master or 5.x installed in editable mode in a conda env, unless I'm asked to test something specific with the installers). Aside from that, I'd think it would be best to test the full installers rather than the lite ones in CI, since the latter are more likely to reveal issues than the former. But again, I'd defer to the expertise of you three on that, who are way more involved than I am on this.

No idea if Github provides stats for that.

AFAIK no, if they are just Action artifacts as opposed to release artifacts.

@mrclary
Copy link
Contributor Author

mrclary commented Jun 11, 2022

Okay, so it sounds like we are converging on a consensus for the following:

  • Pull requests will build only the full version, not the lite version. This will ensure maximum testing coverage, even though it is a larger download.
  • Releases will build both the full and the lite versions
  • Numpy and Pandas will be included in the lite version. This will ensure full capabilities in the Variable Explorer for the lite version.

@CAM-Gerlach
Copy link
Member

Numpy and Pandas will be included in the lite version. This will ensure full capabilities in the Variable Explorer for the lite version.

Seems reasonable enough, if others agree; initially I presumed that their dependencies would substantially narrow the size gap between the two versions (and thus the consequent benefit of the Lite version), but at least my rough testing suggested that is not the case, and it only modestly increases the total needed.

Also, just checking—is Matplotlib in Spyder's runtime env necessary for the Plots pane to work, or only for plots in the variable explorer? And if it is, can it use Matplotlib in the working env instead? I should know but its been a while since I've used Spyder without it.

@mrclary
Copy link
Contributor Author

mrclary commented Jun 12, 2022

Also, just checking—is Matplotlib in Spyder's runtime env necessary for the Plots pane to work, or only for plots in the variable explorer? And if it is, can it use Matplotlib in the working env instead? I should know but its been a while since I've used Spyder without it.

So it appears that for the Lite version (no matplotlib in Spyder's runtime environment), plots appear in the Plots pane just fine. I can set a figure variable (fig = plt.gcf()) and it appears in the Variable Explorer for both versions, however, the Object Inspector does not behave the same.
For the lite version:
Screen Shot 2022-06-11 at 5 15 34 PM
For the full version:
Screen Shot 2022-06-11 at 5 24 16 PM

So should we also be including matplotlib in the Lite builds? Where do we draw the line between Lite and Full functionality for the Object Inspector? Is it possible for the Object Inspector (and Variable Explorer) to use the IPython Console environment for its "inspection" actions? E.g. jupyter run --existing=<existing kernel> <inspection commands>?

@CAM-Gerlach
Copy link
Member

I tested this, but adding Matplotlib to the mix resulted in 230 packages for a total of 285 MB, only around ≈4 fewer packages (though a modest chunk less space) than also requiring sympy, cython and scipy. At that point, you may as well not have the lite install at all, I'd think, and if the benefit to adding Matplotlib is simply being able to use the object inspector on figure and axis objects (which for normal users is likely to be very rare, if they even use MPL's OO API at all versus the functional API) the benefit seems extremely slim, especially considering there are many other custom classes the user is likely to be using that would be more useful to support in the object inspector. So I'd lean strongly against this.

The one other possible benefit, which I don't see above, is whether it has any effect on being able to create plots by right-clicking numpy, pandas, etc. objects in the Variable Explorer—I can't remember if that runs in the working env or Spyder's runtime env, but I'd guess its the former, in which case it would have no effect. That's certainly a lot more benefit than being able to object inspect plot objects, but still not enough to justify it over scipy, cython or sympy, given the space requirements.

Of course, being able to install/update packages in the standalone runtime environment would mostly obviate these tradeoffs, as well as have many other benefits (update Spyder, install plugins, install other packages for Variable Explorer support, etc, without manually bundling them or switching to Anaconda). I recall @ccordoba12 mentioning that someone (@dalthviz ?) was working on exploring that using Micromamba, but I'm not sure the status of that.

@mrclary
Copy link
Contributor Author

mrclary commented Jun 12, 2022

Of course, being able to install/update packages in the standalone runtime environment would mostly obviate these tradeoffs, as well as have many other benefits (update Spyder, install plugins, install other packages for Variable Explorer support, etc, without manually bundling them or switching to Anaconda). I recall @ccordoba12 mentioning that someone (@dalthviz ?) was working on exploring that using Micromamba, but I'm not sure the status of that.

I'm the one looking into that. At this point, micromamba is now included in both the Mac and Windows installers but currently only used for activating conda environments for IPython Console. I plan to explore using the user site packages feature of Python for Spyder's runtime environment and having the user site package directory set to a dedicated directory in Spyder's configuration directory. If this works, then yes, installing/uninstalling plugins for Spyder could easily be accommodated. However, ideally and assuming it is possible, it would still be best to use the matplotlib, pandas, etc from the IPython Console environment for the Variable Explorer and Object Inspector; this would ensure that there aren't any conflicts (however minor). This would also suggest the opportunity for Object Inspector to inspect objects from any package in the user's environment.

@ccordoba12
Copy link
Member

ccordoba12 commented Jun 12, 2022

  • Pull requests will build only the full version, not the lite version. This will ensure maximum testing coverage, even though it is a larger download.
  • Releases will build both the full and the lite versions

I agree with these points.

  • Numpy and Pandas will be included in the lite version. This will ensure full capabilities in the Variable Explorer for the lite version.

This is really up to you @mrclary. I mean, that's because you came up with the concept of Lite releases.

Of course, being able to install/update packages in the standalone runtime environment would mostly obviate these tradeoffs, as well as have many other benefits

That would require building the installers with conda, which is a huge undertaking and a no go at this moment. Besides, if people have a way to install more packages alongside Spyder in our installers, they will end breaking them for sure.

@CAM-Gerlach
Copy link
Member

That would require building the installers with conda, which is a huge undertaking and a no go at this moment.

Oh, I thought you said that one of the devs was exploring Micromamba for this? But maybe I'm mistaken.

Besides, if people have a way to install more packages alongside Spyder in our installers, they will end breaking them for sure.

It would require a narrowly tailored, Spyder-managed interface that only allowed doing very specific, constrained transactions, rather than people being able to (easily) go in and install whatever they wanted, which could be provided by the package management frontend we've been discussing for Spyder 6 (?). However, that's obviously some ways off, in the best case.

@mrclary
Copy link
Contributor Author

mrclary commented Jun 12, 2022

That would require building the installers with conda, which is a huge undertaking and a no go at this moment.

Oh, I thought you said that one of the devs was exploring Micromamba for this? But maybe I'm mistaken.

Besides, if people have a way to install more packages alongside Spyder in our installers, they will end breaking them for sure.

It would require a narrowly tailored, Spyder-managed interface that only allowed doing very specific, constrained transactions, rather than people being able to (easily) go in and install whatever they wanted, which could be provided by the package management frontend we've been discussing for Spyder 6 (?). However, that's obviously some ways off, in the best case.

Okay, I think there are a few conflated concepts here. We should include these topics in our next vision/developer meeting, but I'll try to clarify at least what I envision, though it is not vetted and I don't imply that anyone has endorsed this idea in full or in part.

  • For the macOS app, a dedicated directory inside Spyder's configuration directory could be used for the user site-packages functionality of Python to extend Spyder's runtime environment for "third party" plugins. Alternatively, it may be the possible to install/uninstall "third party" plugins inside the macOS bundle. In either case, these would have to be "managed" by pip and limited to plugins; any packages for what we currently refer to as "Same as Spyder" IPython Console environments would not be handled in this fashion.
  • An environment manager plugin will be developed in the future that allows the user to create/remove/manage virtual environments for use by IPython Console. The primary mechanism here will be micromamba, which is now included in the standalone applications, although it may be extended to include pip and venv. It is my opinion that when this is developed, the need for a "Same as Spyder" environment and the distinction between "Lite" and "Full" version will be obsolete. Spyder will always be a "Lite" version and include only what it needs to operate; there is no need to include additional packages for the user since the user will never interact with it. Rather, there will be a "Default" environment that Spyder may create at first launch (perhaps located in Spyder's config directory somewhere). This will replace the current "Same as Spyder" environment, providing new users a pre-defined environment that they did not have to build, which they can use "out-of-the-box", and yet extend if they wish without affecting Spyder's runtime environment.

There are, of course, implementation details that need to be vetted, such as issues regarding the Variable Explorer and Object Inspector. The above was simply intended to illustrate what I think a final paradigm might look like.

@mrclary mrclary force-pushed the lite-on-pr branch 2 times, most recently from 9b5ee4a to 7f71484 Compare June 14, 2022 20:51
@ccordoba12 ccordoba12 changed the title PR: Only build Lite macOS app on pull requests PR: Only build Full macOS app on pull requests Jun 14, 2022
@ccordoba12 ccordoba12 added this to the v5.3.2 milestone Jun 14, 2022
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mrclary!

Note: The failure in our tests is unrelated to the Mac installer.

@ccordoba12 ccordoba12 merged commit 2857ee8 into spyder-ide:5.x Jun 15, 2022
ccordoba12 added a commit that referenced this pull request Jun 15, 2022
@mrclary
Copy link
Contributor Author

mrclary commented Jun 15, 2022

@ccordoba12 I appreciate your prompt merging, but I found an error in the GitHub actions, so the actual macOS application did not get built. I'll submit a follow-up PR. Sorry, I should have mentioned it.

FYI, I reverted the additions of pandas and numpy to the Lite build because it seemed a little vague still regarding the where we wanted to draw the line with regard to Variable Explorer and Object Explorer functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants