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

Support Python 3.12 with CI, move tests to top level #89

Merged
merged 6 commits into from
Sep 30, 2024

Conversation

bobleesj
Copy link

Closes #88

Support Python 3.12 for diffpy.srreal, a major bump from 3.7.

How to reproduce locally:

conda create -n diffpy.srfit_312 python=3.12
conda activate diffpy.srfit_312
pip install -r requirement/run.txt
pip install . --no-deps
pytest

====== 80 passed, 30 skipped, 1 warning in 0.43s =====

Time to cookiecut next, once merged.

@bobleesj
Copy link
Author

Not sure why CI isn't running here since I've manually added tests-on-pr.yml

But runs using my forked CI:

Screenshot 2024-09-26 at 1 05 50 AM

@sbillinge ready for review

@bobleesj bobleesj marked this pull request as ready for review September 26, 2024 05:06
Copy link
Author

Choose a reason for hiding this comment

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

Providing just a basic setup to build from the sources.

@@ -0,0 +1,3 @@
scipy
numpy
matplotlib
Copy link
Author

@bobleesj bobleesj Sep 26, 2024

Choose a reason for hiding this comment

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

we could add diffpy.structure while/after we address the issue #90

Copy link
Contributor

Choose a reason for hiding this comment

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

yes. I think it is ok to add those other dependencies. For this particular package we could, if we want, have it split out into run-core.txt and ren-ext.txt? Strictly, srfit could be used as a general fitting program in a more or less stand-alone way, (core) but practically, it is always used with srreal and structure. What do you think?

Copy link
Author

@bobleesj bobleesj Sep 27, 2024

Choose a reason for hiding this comment

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

@sbillinge

Can we have the core packages underrun.txt and just create run_ext.txt for the extra?

Since our centralized GH CI workflow files need to be updated:

from:

      - name: Install ${{ inputs.project }} and requirements
        run: |
          conda install --file requirements/run.txt

to:

      - name: Install ${{ inputs.project }} and requirements
        run: |
          conda install --file requirements/run.txt
          conda install --file requirements/run-ext.txt

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, good idea

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.

I reviewed this but didn't finish and submit the review, but here it is

@@ -0,0 +1,3 @@
scipy
numpy
matplotlib
Copy link
Contributor

Choose a reason for hiding this comment

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

yes. I think it is ok to add those other dependencies. For this particular package we could, if we want, have it split out into run-core.txt and ren-ext.txt? Strictly, srfit could be used as a general fitting program in a more or less stand-alone way, (core) but practically, it is always used with srreal and structure. What do you think?

@bobleesj
Copy link
Author

@sbillinge Could you please merge this to cookie as shown above? In the following PRs, I will re-cookiecut and address the skipped tests.

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