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

Switch to src layout with separate tests directory #2598

Merged
merged 122 commits into from Apr 12, 2024

Conversation

namurphy
Copy link
Member

@namurphy namurphy commented Mar 26, 2024

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.

@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 For linters (e.g., flake8, ruff) and autoformatters (e.g., ruff, black, isort, Sourcery) labels Mar 26, 2024
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@@ -12,15 +12,13 @@ include *.yaml
include *.yml
include changelog/README.rst
include CITATION.cff
include plasmapy/py.typed
Copy link
Member Author

@namurphy namurphy Mar 26, 2024

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.

Copy link
Member Author

@namurphy namurphy Mar 26, 2024

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.

Copy link

codecov bot commented Mar 26, 2024

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.plasma Related to the plasmapy.plasma subpackage plasmapy.formulary Related to the plasmapy.formulary subpackage 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
Copy link
Member Author

namurphy commented Apr 12, 2024

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/.

@namurphy namurphy merged commit 2a6cf7f into PlasmaPy:main Apr 12, 2024
17 checks passed
@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
@pheuer
Copy link
Member

pheuer commented Apr 12, 2024

@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
Copy link
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!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 For linters (e.g., flake8, ruff) and autoformatters (e.g., ruff, black, isort, Sourcery) 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
Development

Successfully merging this pull request may close these issues.

Switch to an src layout with a separate tests directory?
3 participants