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

Improve organization #485

Merged
merged 21 commits into from
Aug 13, 2024
Merged

Improve organization #485

merged 21 commits into from
Aug 13, 2024

Conversation

CodyCBakerPhD
Copy link
Contributor

Motivation

@stephprince Here is the first refactor for improved package navigation

This implements modern sklearn-style private vs. public imports and properly scoped submodules (no things like from nwbinspector.utils import os allowed)

No actual code has changed (+/- marking some very low level functions as private or public), just moving things around

CodyCBakerPhD added 2 commits August 12, 2024 13:37
@CodyCBakerPhD CodyCBakerPhD self-assigned this Aug 12, 2024
@CodyCBakerPhD
Copy link
Contributor Author

@stephprince I will ping you again once the internal routing is all unscrambled/debugged

@CodyCBakerPhD CodyCBakerPhD marked this pull request as ready for review August 13, 2024 17:18
@CodyCBakerPhD
Copy link
Contributor Author

@stephprince OK this is ready now if you want to take a look before our meeting

In the CHANGELOG I have emphasized what is being hard vs. soft deprecated

Most classical imports from the API should still work with a deprecation warning

Anything that this PR 'breaks' in terms of back-compatibility is for things that never should have been imported by users to begin with (various private functions we're not held accountable for exposing, third-party packages, etc.)

Two follow-up PRs to this will include a similar treatment for the checks submodule pending some discussion of how magical we want it to be (to make it easier for developers to write new check functions) as well as a ruff linter for automatic import ordering/cleaning

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@stephprince stephprince left a comment

Choose a reason for hiding this comment

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

Looks good to me!

src/nwbinspector/register_checks/__init__.py Outdated Show resolved Hide resolved
src/nwbinspector/version/__init__.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 83.19328% with 40 lines in your changes missing coverage. Please review.

Project coverage is 85.55%. Comparing base (6fc57e5) to head (87233e4).

Files Patch % Lines
src/nwbinspector/_inspection_cli.py 81.35% 11 Missing ⚠️
src/nwbinspector/_formatting.py 53.33% 7 Missing ⚠️
src/nwbinspector/inspector_tools/__init__.py 0.00% 5 Missing ⚠️
src/nwbinspector/nwbinspector/__init__.py 0.00% 5 Missing ⚠️
src/nwbinspector/register_checks/__init__.py 0.00% 5 Missing ⚠️
src/nwbinspector/version/__init__.py 0.00% 4 Missing ⚠️
src/nwbinspector/_configuration.py 96.36% 2 Missing ⚠️
src/nwbinspector/_types.py 96.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #485      +/-   ##
==========================================
- Coverage   87.14%   85.55%   -1.60%     
==========================================
  Files          23       33      +10     
  Lines        1260     1308      +48     
==========================================
+ Hits         1098     1119      +21     
- Misses        162      189      +27     
Flag Coverage Δ
unittests 85.55% <83.19%> (-1.60%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/nwbinspector/_inspection.py 83.08% <100.00%> (ø)
src/nwbinspector/_organization.py 100.00% <100.00%> (ø)
src/nwbinspector/_registration.py 98.33% <100.00%> (ø)
src/nwbinspector/_version.py 100.00% <ø> (ø)
src/nwbinspector/checks/__init__.py 100.00% <100.00%> (ø)
src/nwbinspector/checks/behavior.py 100.00% <100.00%> (ø)
src/nwbinspector/checks/ecephys.py 100.00% <100.00%> (ø)
src/nwbinspector/checks/general.py 100.00% <100.00%> (ø)
src/nwbinspector/checks/icephys.py 100.00% <100.00%> (ø)
src/nwbinspector/checks/image_series.py 93.93% <100.00%> (ø)
... and 20 more

... and 1 file with indirect coverage changes

@CodyCBakerPhD CodyCBakerPhD merged commit d6a13ca into dev Aug 13, 2024
36 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the improve_organization branch August 13, 2024 21:17
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