Skip to content

Support 3.12 CI - conda install wxPython, pip install gooey - to prevent building wxPython #137

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

Merged
merged 4 commits into from
Dec 3, 2024

Conversation

bobleesj
Copy link
Contributor

No description provided.

Copy link

Warning! No news item is found for this PR. If this is a user-facing change/feature/fix,
please add a news item by copying the format from news/TEMPLATE.rst.

@bobleesj
Copy link
Contributor Author

(Not ready for review)

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.31%. Comparing base (a2572da) to head (c8ae738).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #137   +/-   ##
=======================================
  Coverage   99.31%   99.31%           
=======================================
  Files           6        6           
  Lines         293      293           
=======================================
  Hits          291      291           
  Misses          2        2           

@@ -13,7 +13,7 @@ jobs:
run:
shell: bash -l {0}

runs-on: ubuntu-latest
runs-on: macos-latest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out wxpython needs to be built for linux since no native whl is provided while we have windows/macos whl files provided shown below:

Screenshot 2024-11-27 at 4 03 15 PM

Since @yucongalicechen and I test using macOS, what if we setup a MacOS CI instead?

Ref:
https://pypi.org/project/wxPython/#files

Copy link
Contributor

Choose a reason for hiding this comment

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

Move back to Linux after we add wxpython to conda.txt

@@ -2,4 +2,3 @@ numpy
diffpy.utils
pandas
scipy
gooey
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed gooey from conda.txt since we won't be able to install it and it will only fail the CI during conda install.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to add wxpython to conda.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, done.

@bobleesj
Copy link
Contributor Author

@sbillinge ready for review. pls see in-line comments. Thank you.

@bobleesj
Copy link
Contributor Author

@yucongalicechen tagging Yucong as well to share what's going on here.

@bobleesj bobleesj changed the title Remove gooey in conda.txt and install gooey via pip in CI Remove gooey in conda.txt and install gooey via pip in CI, setup macOS CI to prevent building wxPython Nov 27, 2024
Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

Does adding wxpython to conda.txt allow you to set it back to Linux-latest?

@bobleesj
Copy link
Contributor Author

Does adding wxpython to conda.txt allow you to set it back to Linux-latest?

I checked wxpython's conda-forge files, no support for py313 as of now:

Screenshot 2024-11-28 at 8 44 26 AM

So it will most likely break the CI while installing it.

Ref: https://anaconda.org/conda-forge/wxpython/files

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

It might be cleaner to close this and redo following the regolith protocol

@@ -2,4 +2,3 @@ numpy
diffpy.utils
pandas
scipy
gooey
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to add wxpython to conda.txt

@@ -13,7 +13,7 @@ jobs:
run:
shell: bash -l {0}

runs-on: ubuntu-latest
runs-on: macos-latest
Copy link
Contributor

Choose a reason for hiding this comment

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

Move back to Linux after we add wxpython to conda.txt

@@ -32,15 +32,11 @@ jobs:
conda config --set always_yes yes
--set changeps1 no

- name: Install libgtk for Linux
Copy link
Contributor

Choose a reason for hiding this comment

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

Restore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

libgtk is no longer needed. here, wxpython is install via conda-forge as suggested.

@bobleesj bobleesj changed the title Remove gooey in conda.txt and install gooey via pip in CI, setup macOS CI to prevent building wxPython Support 3.12 CI - conda install wxPython, pip install gooey - to prevent building wxPython Dec 2, 2024
@bobleesj bobleesj changed the title Support 3.12 CI - conda install wxPython, pip install gooey - to prevent building wxPython Support 3.12 CI - conda install wxPython, pip install gooey - to prevent building wxPython Dec 2, 2024
@bobleesj
Copy link
Contributor Author

bobleesj commented Dec 2, 2024

@sbillinge ready for review - thanks for the suggestion. wxpython is conda-installed as suggested while gooey is pip installed.

@sbillinge sbillinge merged commit 5f0c1b4 into diffpy:main Dec 3, 2024
4 of 5 checks passed
@sbillinge
Copy link
Contributor

nice. Much cleaner.

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.

2 participants