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

Bump scipy and remove requirements.txt #51

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

loeiten
Copy link
Member

@loeiten loeiten commented Jul 19, 2023

No description provided.

@loeiten loeiten requested a review from ZedThree July 19, 2023 07:34
@dschwoerer
Copy link
Contributor

I am against blocking users of old scipy from installing boututils 👎

I think it is an unreasonable burden to get a recent scipy installing just for reading some bout++ simulations.

Same applies to boutproject/boutdata#94

I did intentionally not bump it.

Looking at:
https://ubuntu.com/security/notices/USN-6226-1

They require local access - if you can anyway can run python code - why bother with the vulnerability in the first place?
scipy/scipy#16235 seems to agree that this is not a security vulnerability.

@loeiten
Copy link
Member Author

loeiten commented Jul 19, 2023

I agree that fixing this vulnerability is less severe than others.

I'm not sure what burden there is for users to install a newer update of a python package. Maybe I'm missing something here, but if updating a package is an issue maybe it points to a bigger underlying issue of packaging/installing the software?

At some point a more severe vulnerability might be uncovered, and as updates to these packages usually also includes performance enhancements I think it would be a good practice to occasionally update the packages.

@dschwoerer
Copy link
Contributor

I generally try to use quite recent versions, so this will not impact my workflows.

However, if there are no pre-compiled binaries for your python version, you can either start installing python your self on an HPC system - and then you have to make sure to use a compatible compiler, or you need to compile dependencies your self, figuring out why compiling fails, manually install headers, which are not meant to be installed by a user. This can be a huge time sink. I had to do this for some systems. Forcing any body to do this for the reason of "performance improvments" of maybe a few milliseconds, is just not worth it if it can cost days for a user to get stuff working.
I guess they will likely (hopefully) just fall back to the previous boutdata/boututils version - but it is not at all needed, we did not change compatibility.

Sure, people should update there system. I am working on getting python 3.12 working for xbout (bout++ has already been rebuild for python3.12)- but that does not mean we should make users live hard, if they cannot do so easily.

Filing tickets with HPC systems that provide ancient stuff would imho be a better use of our time.

@loeiten
Copy link
Member Author

loeiten commented Jul 20, 2023

you can either start installing python your self on an HPC system

Ahh...brings back memories 😅

I am working on getting python 3.12 working for xbout

Nice 🙂

that does not mean we should make users live hard

I agree. I think there is a world where security and good user experience can live together in harmony. It's unfortunate that the state of HPC systems are what they are, and I agree that this is ultimately not BOUT's responsibility (although I think everyone should work towards creating secure solutions). In an ideal world one could perhaps containerise the projects and by that have packages up to date.

I'll update the commit to just remove the requirements file.

@dschwoerer
Copy link
Contributor

@loeiten Are you still going to update this PR?

@loeiten
Copy link
Member Author

loeiten commented Apr 22, 2024

No, I totally forgot about this. Please commandeer if you see fit 😊

pyproject.toml Outdated Show resolved Hide resolved
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