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

Update to package configuration #39

Merged
merged 14 commits into from
Jan 28, 2022

Conversation

LiamPattinson
Copy link
Contributor

This update aims to bring openmc-plasma-source in line with PEP 517 recommendations for packaging etc. The build/install details in setup.py are moved to setup.cfg and pyproject.toml as simple input files. Rather than calling:

  • python setup.py install/python setup.py develop
  • python setup.py sdist bdist_wheel

One should instead call:

  • pip install ./pip install -e . (for 'develop' mode)
  • python -m build

Rather than using a requirements file, the project requirements may be listed in setup.cfg, and optional dependencies may be installed using:

  • pip install -e .[tests] (-e is optional)

This update also introduces automatic version inference, meaning the git tag is the 'single source of truth' for versioning. The version is updated with developer tags when between patch versions, i.e. one commit after 0.5.3 will be 0.5.4.dev0+[git commit].d[date].

@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #39 (4889184) into develop (ca46cab) will decrease coverage by 2.66%.
The diff coverage is 78.57%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #39      +/-   ##
===========================================
- Coverage    96.34%   93.67%   -2.67%     
===========================================
  Files            6        6              
  Lines          164      174      +10     
===========================================
+ Hits           158      163       +5     
- Misses           6       11       +5     
Impacted Files Coverage Δ
openmc_plasma_source/plotting/__init__.py 100.00% <ø> (ø)
openmc_plasma_source/__init__.py 61.53% <50.00%> (-38.47%) ⬇️
openmc_plasma_source/point_source.py 86.66% <80.00%> (ø)
openmc_plasma_source/ring_source.py 88.88% <80.00%> (ø)
openmc_plasma_source/tokamak_source.py 97.87% <90.00%> (ø)
...enmc_plasma_source/plotting/plot_tokamak_source.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a09eb2...4889184. Read the comment docs.

@shimwell
Copy link
Member

Great to have this all in one place. Looking forward to making use of this elsewhere as well. @RemDelaporteMathurin we have a similar PR on the Paramak that has been merged already and I think this one is good to go if you are happy with it

@RemDelaporteMathurin
Copy link
Member

@shimwell @LiamPattinson thanks again for this. For some reason the CI/ testing workflow failed, does it need updating too?

@LiamPattinson
Copy link
Contributor Author

Going to be honest, I'm really not sure why that failed. It's odd that pytest ran fine, but the python calls didn't. All I can think of is that circleci might be using Python 3 for pip calls, and Python 2.7 for direct python calls. I've made it explicit, and we'll see how it goes.

@LiamPattinson
Copy link
Contributor Author

I had to play around in the openmc docker image to get to the root of this, and I think it might be as simple as doing a proper install rather than a 'develop' install in the CI. Calling pip install -e . in a locally hosted docker container led to similar issues of openmc-plasma-source not being included in the python path.

@RemDelaporteMathurin
Copy link
Member

@LiamPattinson for some reason there seem to be some conflicts with this PR and I can't run the workflow until there are fixed

@LiamPattinson
Copy link
Contributor Author

Sorry about that, the latest fix seems to have done it.

Copy link
Member

@RemDelaporteMathurin RemDelaporteMathurin left a comment

Choose a reason for hiding this comment

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

Thanks for this @LiamPattinson

@RemDelaporteMathurin RemDelaporteMathurin merged commit ecce6d4 into fusion-energy:develop Jan 28, 2022
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