Skip to content

Switch to src layout with separate tests directory - #2598

Merged
namurphy merged 122 commits into
PlasmaPy:mainfrom
namurphy:src-of-inspiration
Apr 12, 2024
Merged

Switch to src layout with separate tests directory#2598
namurphy merged 122 commits into
PlasmaPy:mainfrom
namurphy:src-of-inspiration

Conversation

@namurphy

@namurphy namurphy commented Mar 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Description

This PR changes the layout to be aligned with what is now recommended in the PyPA Python Packaging Guide. The following changes are made:

  • The plasmapy/ directory has been changed to src/plasmapy/
  • Tests have been moved to a separate top-level tests/ directory.
  • The contributor guide has been updated to reflect changes in the locations of things

Below I included my suggestions for other projects on switching to an src layout.

Motivation and context

See #2581 for the motivation. Essentially, the src layout can prevent a variety of easy-to-overlook problems. My hope is to get this merged well ahead of the summer school, since we're hoping to welcome new & potential contributors.

Code review checklist

While this PR says it modifies ∼239 files 😱, nearly all of the files were moved without being changed and don't require review. 😮‍💨 The files that warrant the most attention are:

  • docs/contributing/testing_guide.rst
  • changelog/2598.internal.rst
  • pyproject.toml
  • tests/__init__.py

Thank you! 🙏🏻

Task checklist

  • git mv plasmapy src/plasmapy
  • Move tests to tests/
  • Update .gitignore paths
  • Update & consolidate paths in MANIFEST.in
  • Update paths in .pre-commit-config.yaml
  • Update paths in contributor guide
  • Describe location of source code & tests in contributor guide
  • Update/regenerate per-file ignores in mypy.ini
  • pyproject.toml: update setuptools & setuptools_scm paths
  • pyproject.toml: update testpaths in pytest
  • pyproject.toml: update coverage settings (define source and update paths in omit)
  • Verify that code coverage is unchanged
  • pyproject.toml: in isort settings, add plasmapy as known-first-party
  • pyproject.toml: update ruff per-file ignores
  • Update paths included in comments throughout the package
  • tox.ini: update pytest command
  • tox.ini: Update mypy command

Related issues

Closes #2581.

namurphy added 18 commits March 26, 2024 15:03
@namurphy
Update .gitignore for src
ba7a7c8
@namurphy
git mv plasmapy src/
e28be57
@namurphy
Update pyproject.toml for src
b315c20
@namurphy
Update mypy command
470c1f2
@namurphy
Update .pre-commit-config.yaml
a61c593
@namurphy
Temporarily add script to move tests to tests/
7360a1f
@namurphy
Remove temporary script
9f450ae
@namurphy
Update MANIFEST.in
138ac85
@namurphy
Update paths for ruff ignore
d773d55
@namurphy
Update ruff per-file ignores
a2331fd
@namurphy
Ignore I001 in notebooks temporarily (for this PR)
7ff3ded
@namurphy
Update pytest commands
dd87dd6
@namurphy
Change directory for pytest command
bcc0eca
@namurphy
Update location of tests in testing guide
b4afd1b
@namurphy
Update file paths to src/plasmapy/...
7ccafad
@namurphy
Ignore ruff rule W291
5d13a40 
This one is to ignore trailing whitespace, but this is handled by
ruff-format and so is unnecessary. My motivation for removing this
rule was because enabling the ruff extension for PyCharm made it
so that trailing whitespace was highlighted, even while code was
still being written.
@namurphy
Update file locations in doc guide
bed8405
@namurphy
Update file paths in coding guide
50a8910
@github-actions github-actions Bot added docs PlasmaPy Docs at http://docs.plasmapy.org testing CI Related to continuous integration contributor guide packaging Related to packaging or distribution linters Code linters and autoformatters labels Mar 26, 2024
namurphy
namurphy commented Mar 26, 2024
View reviewed changes
Comment thread .pre-commit-config.yaml Resolved
namurphy
namurphy commented Mar 26, 2024
View reviewed changes
Comment thread MANIFEST.in
include *.yml
include changelog/README.rst
include CITATION.cff
include plasmapy/py.typed

@namurphy namurphy Mar 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

📝 This line is probably not necessary because everything within src is now included.

namurphy
namurphy commented Mar 26, 2024
View reviewed changes
Comment thread MANIFEST.in

@namurphy namurphy Mar 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

❔ I'm not sure if we want to include tests/ in MANIFEST.in. I'd like the tests to be included in source distributions for completeness, but not included in wheels.

namurphy
namurphy commented Mar 26, 2024
View reviewed changes
Comment thread docs/contributing/testing_guide.rst Resolved
@codecov

codecov Bot commented Mar 26, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.18%. Comparing base (923ab22) to head (2c311a0).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2598   +/-   ##
=======================================
  Coverage   95.17%   95.18%           
=======================================
  Files         104      103    -1     
  Lines        9417     9412    -5     
  Branches     2154     2153    -1     
=======================================
- Hits         8963     8959    -4     
  Misses        276      276           
+ Partials      178      177    -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@namurphy namurphy added status: ready for review PRs that are ready for code review refactoring ♻️ Improving an implementation without adding new functionality and removed plasmapy.diagnostics Related to the plasmapy.diagnostics subpackage plasmapy.utils Related to the plasmapy.utils subpackage plasmapy.dispersion Related to the plasmapy.dispersion subpackage notebooks Related to example Jupyter notebooks in docs/examples/ plasmapy.analysis Related to the plasmapy.analysis subpackage plasmapy.simulation Related to the plasmapy.simulation subpackage labels Apr 9, 2024
@github-actions github-actions Bot added the plasmapy.particles Related to the plasmapy.particles subpackage label Apr 12, 2024
@namurphy

namurphy commented Apr 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Preview of admonition from GitHub Action to post on PRs:

Note

PlasmaPy recently switched to an src layout. The source code previously in plasmapy/ is now in src/plasmapy/. Tests are now in tests/.

rocco8773
rocco8773 reviewed Apr 12, 2024
View reviewed changes
Comment thread src/plasmapy/_dev/scm_version.py Resolved
@namurphy
namurphy merged commit 2a6cf7f into PlasmaPy:main Apr 12, 2024
@namurphy
namurphy deleted the src-of-inspiration branch April 12, 2024 19:13
@github-actions github-actions Bot removed the status: ready for review PRs that are ready for code review label Apr 12, 2024
@namurphy namurphy mentioned this pull request Apr 12, 2024
Closed
@pheuer

pheuer commented Apr 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

@namurphy FYI I had to re-do the editable installation in order to get internal imports working again after this merge, e.g. import plasmapy worked but from plasmapy.diagnostics import thomson did not until I re-ran pip install -e . Assuming there's no way around this, we may want to prominently note that this needs to be done?

Also, the plasmapy folder is still there (I guess Git doesn't delete the folder) so people may be confused looking for where the source code went if they aren't following the PRs

@namurphy

namurphy commented Apr 12, 2024

Copy link
Copy Markdown
Member Author

@pheuer — thank you for the follow-up! I'll add something to README.md noting what you said.

(Also I'm at UCLA working next to @rocco8773 today, and just saw two of your posters in the hallway here!)

@namurphy namurphy mentioned this pull request Apr 12, 2024
Merged
@namurphy namurphy mentioned this pull request Apr 29, 2024
Merged
This was referenced May 9, 2024
Open
Merged
@namurphy namurphy mentioned this pull request May 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rocco8773 rocco8773 rocco8773 approved these changes
@pheuer pheuer Awaiting requested review from pheuer pheuer is a code owner
@ejohnson-96 ejohnson-96 Awaiting requested review from ejohnson-96

Assignees

No one assigned

Labels

CI Related to continuous integration contributor guide docs PlasmaPy Docs at http://docs.plasmapy.org GitHub Actions A continuous integration platform for automating tests and other tasks (see .github/ directory) linters Code linters and autoformatters packaging Related to packaging or distribution plasmapy.particles Related to the plasmapy.particles subpackage priority: high Issues & PRs with significant urgency and importance that should be addressed soon refactoring ♻️ Improving an implementation without adding new functionality testing

Projects

None yet

Milestone

v2024.5.0

Development

Successfully merging this pull request may close these issues.

Switch to an src layout with a separate tests directory?

3 participants

@namurphy @pheuer @rocco8773