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

Remove imports from gsl.all #38874

Merged
merged 2 commits into from
Dec 8, 2024
Merged

Remove imports from gsl.all #38874

merged 2 commits into from
Dec 8, 2024

Conversation

tobiasdiez
Copy link
Contributor

I had troubles with importing gsl (on Windows) and tried to fix it by changing imports from gsl.all to more specific imports. Turns out the real problem was actually wrong pkg-config entry; but I thought these more specific imports might be helpful anyway and should (slightly) reduce the size of the built library.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

Copy link

github-actions bot commented Oct 28, 2024

Documentation preview for this PR (built with commit ee8582e; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 30, 2024

Otherwise lgtm.

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 30, 2024

By the way, the "Build & Test using Conda/Meson" would be fixed soon? Please let Checks fail only for issues of the PR itself.

tobiasdiez and others added 2 commits November 18, 2024 11:37
I had troubles with importing `gsl` (on Windows) and tried to fix it by changing imports from `gsl.all` to more specific imports. Turns out the real problem was actually wrong `pkg-config` entry; but I thought these more specific imports might be helpful anyway and should (slightly) reduce the size of the built library.
Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

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

LGTM.

vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 6, 2024
    
I had troubles with importing `gsl` (on Windows) and tried to fix it by
changing imports from `gsl.all` to more specific imports. Turns out the
real problem was actually wrong `pkg-config` entry; but I thought these
more specific imports might be helpful anyway and should (slightly)
reduce the size of the built library.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [ ] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38874
Reported by: Tobias Diez
Reviewer(s): Kwankyu Lee, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 8, 2024
    
I had troubles with importing `gsl` (on Windows) and tried to fix it by
changing imports from `gsl.all` to more specific imports. Turns out the
real problem was actually wrong `pkg-config` entry; but I thought these
more specific imports might be helpful anyway and should (slightly)
reduce the size of the built library.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [ ] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38874
Reported by: Tobias Diez
Reviewer(s): Kwankyu Lee, Tobias Diez
@vbraun vbraun merged commit 1b38e02 into sagemath:develop Dec 8, 2024
21 of 23 checks passed
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.

3 participants