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

Add pip support #175

Merged
merged 22 commits into from
May 4, 2022
Merged

Conversation

DavidAnderegg
Copy link
Contributor

Overview

As promised in #171 this PR makes SHARPy installable via pip. After building and testing, you simply install it in the current anaconda environment by running pip install .. This copies all necessary files into the environment and makes the command sharpy globally available (as long as the anaconda environment is activated).

If you are developing SHARPy, you can install it in development mode with this command: pip install -e .. This creates only a link to the location of the SHARPy files and allows "live editing" of its source while still having all the other benefits. More about it here.

Changes

  • created setup.pywhich handles the install procedure (you might want to make sure I entered the proper information)
  • added __version__ variable to sharpy.__init__.py. This tells the setup the current version of the library and has to be changed manually before a new release
  • moved the cases folder into sharpy/cases. This way it becomes part of the sharpy package itself. I guess this is the most controversial change I am proposing. But it is a lot cleaner and makes life easier (the examples can be imported everywhere via sharpy.cases....). One could argue that it should not be part of the library as it does not really provide functionality. But I have seen this in the past (for example here) and I believe the benefits are worth it. What do you think?
  • Changed the examples to import cases from SHARPy (e.g from sharpy.cases.templates import ...)
  • Removed the bin directory and its content
  • added the function sharpy_run() to sharpy_main.py which gets executed whenever you call sharpy in the console. It is basically the same as bin/sharpy was
  • Updated the docs to remove mentions of sharpy_vars.sh

General Notes

  • I have broken the example nonlinear_t-tail_HALE.ipynb because i moved the cases folder. Maybe you can fix it? All the other examples seem to work.
  • Please don't do this again. This completely breaks expected python behavior and took me quite some time to figure out.

I am quite sure i broke something it did not think about. Please test it and let me know what you think. Feel free to commit to my branch.

@DavidAnderegg DavidAnderegg changed the title Dev pipsupport Add pip support Sep 7, 2021
@ngoiz ngoiz self-requested a review September 8, 2021 07:29
@ngoiz ngoiz self-assigned this Sep 8, 2021
@ngoiz
Copy link
Contributor

ngoiz commented Sep 8, 2021

Hi David,

Thank you very much for taking the effort and adding this to SHARPy, it will be a great addition that will make things a lot easier. I'll go through it over the next few days and comment here or commit directly to the branch if I see anything.

Again, many thanks!

@ngoiz ngoiz marked this pull request as ready for review May 3, 2022 14:28
Copy link
Contributor

@ngoiz ngoiz left a comment

Choose a reason for hiding this comment

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

Finally got around to reviewing and updating this, apologies and many thanks for adding this feature, @DavidAnderegg!!

All looks good, I added minor things and restored a data file that was lost thanks to gitignore I assume.

Branch updated with the latest development branch that includes the automated unittests as a Github action that are also passing.

Otherwise status check left pending and unable to merge
@ngoiz ngoiz merged commit 7956fc5 into ImperialCollegeLondon:develop May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants