Skip to content

Add coding convention for internal subpackages without __init__.py files #31618

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

Closed
wants to merge 1 commit into from

Conversation

schrockn
Copy link
Member

@schrockn schrockn commented Aug 7, 2025

Summary & Motivation

This PR codifies a coding convention for internal subpackage organization in the Dagster codebase. It establishes that internal subpackages should not have __init__.py files and should use absolute imports instead.

The changes include:

  • Added explicit guidance in CLAUDE.md stating "DO NOT create __init__.py files for internal subpackages - use absolute imports instead since we use absolute imports throughout the codebase"
  • Updated pyproject.toml ruff configuration to ignore INP001 linting errors for the mixins directory, as it's an internal subpackage that intentionally doesn't have an __init__.py file

This convention aligns with the existing codebase patterns and prevents unnecessary __init__.py files in internal organizational directories.

How I Tested These Changes

Verified that ruff linting passes with the new configuration and that the coding conventions are clearly documented.

Copy link
Member Author

schrockn commented Aug 7, 2025

@schrockn schrockn changed the title claude-guidance Add coding convention for internal subpackages without __init__.py files Aug 7, 2025
@schrockn schrockn force-pushed the schrockn/claude-updates branch from f37cf5e to 693814a Compare August 7, 2025 12:24
@schrockn schrockn force-pushed the schrockn/ir-7-delete-plan branch from 1e869f5 to a8f0d09 Compare August 7, 2025 13:32
@schrockn schrockn force-pushed the schrockn/claude-updates branch from 693814a to 9ef6e80 Compare August 7, 2025 13:32
@schrockn schrockn marked this pull request as ready for review August 7, 2025 13:36
@schrockn schrockn force-pushed the schrockn/ir-7-delete-plan branch from a8f0d09 to a457021 Compare August 7, 2025 15:13
@schrockn schrockn force-pushed the schrockn/claude-updates branch from 9ef6e80 to 14b9e13 Compare August 7, 2025 15:13
@schrockn schrockn force-pushed the schrockn/ir-7-delete-plan branch from a457021 to fc22533 Compare August 7, 2025 15:43
@schrockn schrockn force-pushed the schrockn/claude-updates branch 2 times, most recently from 8b593f4 to f4ee657 Compare August 7, 2025 16:29
@schrockn schrockn force-pushed the schrockn/ir-7-delete-plan branch from bb36051 to 6f73e93 Compare August 7, 2025 17:21
@schrockn schrockn force-pushed the schrockn/claude-updates branch 2 times, most recently from 9054ada to db9c8a6 Compare August 7, 2025 18:17
@schrockn schrockn force-pushed the schrockn/ir-7-delete-plan branch from 6f73e93 to 8884733 Compare August 7, 2025 18:17
@schrockn schrockn requested a review from OwenKephart August 7, 2025 21:40
This commit codifies a coding convention for internal subpackage organization:
- Added explicit guidance in CLAUDE.md about not creating __init__.py files for internal subpackages
- Updated pyproject.toml ruff configuration to ignore INP001 errors for mixins directory
- Aligns with existing codebase patterns of using absolute imports without __init__.py for internal organization
Copy link
Contributor

@OwenKephart OwenKephart left a comment

Choose a reason for hiding this comment

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

I don't think this is a convention that we currently use -- we always have __init__.py files, we just keep them empty for the most part. I think the directive should just be something along the lines of:

ALWAYS keep `__init__.py` files empty

@schrockn schrockn force-pushed the schrockn/claude-updates branch from db9c8a6 to d2d925a Compare August 7, 2025 22:11
@schrockn schrockn force-pushed the schrockn/ir-7-delete-plan branch from 8884733 to b03a8f7 Compare August 7, 2025 22:11
Copy link
Member Author

schrockn commented Aug 7, 2025

yeah this is totally wrong actually. breaks MANIFEST.in

@schrockn schrockn closed this Aug 7, 2025
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.

2 participants