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

refactor: Rework "exported" and "public" logic #281

Closed
5 tasks done
pawamoy opened this issue May 31, 2024 · 0 comments
Closed
5 tasks done

refactor: Rework "exported" and "public" logic #281

pawamoy opened this issue May 31, 2024 · 0 comments
Assignees
Labels
feature New feature or request
Milestone

Comments

@pawamoy
Copy link
Member

pawamoy commented May 31, 2024

Currently, the "exported" logic is used both for expanding wildcard and for knowing if we should check an object for breaking changes. This is not ideal: our "exported" logic is actually only meant for wildcard expansion. See mkdocstrings/python#39 (comment) for context.

Tasks:

  • Rename "exported" to "wildcard exposed" or similar
  • Fix the "exposed" logic to exclude private members (starting with _) when __all__ is undefined
  • Stop making an explicit vs. implicit distinction
  • Use new logic in Object.is_public
  • Use is_public in API checker instead of "exported" logic
@pawamoy pawamoy added the feature New feature or request label May 31, 2024
@pawamoy pawamoy self-assigned this May 31, 2024
@pawamoy pawamoy added this to the 1.0.0 milestone Jun 8, 2024
pawamoy added a commit that referenced this issue Jun 9, 2024
We observed that Griffe's internals/API were a bit confusing, so decided to refactor them.

Griffe has a concept of "exported member". A **module member** is considered:

- **explicitly exported** when its name appears in the module's `__all__`
- **implicitly exported** when the module's `__all__` is undefined, the member is available at runtime (for example not guarded behind `if TYPE_CHECKING`), and the member is an alias (defined elsewhere), *except* if it's a submodule, *unless* it was also imported in the current module

A **class member** is considered **implicitly exported**, the same way as module members are considered implicitly exported. It is never considered explicitly exported because we don't/can't use `__all__` in class bodies.

This "exported" marker is then used in a few places, 3 of of them being of interest to us here.

- if `__all__` is defined in the wildcard imported module, add all objects referenced in the imported module's `__all__` to the importing module
- else, add all objects that are implicitly exported by the imported module to the importing module

There is a first issue here: members starting with `_` are considered implicitly exported. This is wrong: actual wildcard imports will not import these objects at runtime.

Between the same old and same new module, if a member disappeared, and it was either a submodule, or it was exported (explicitly or implicitly), we consider its disappearance a breaking change. Again, it's wrong here to consider that a private object's disappearance makes a breaking change. This issue wasn't spotted before because the code that checks breaking changes has an earlier condition on the object names: it skips private objects anyway.

Griffe objects have an `is_public()` method that tell if an object is public or not. An object is:

- **strictly public** if:
  - its `public` attribute is true
  - or it is explicitly exported
- **public** if:
  - it is strictly public
  - or it's not private (name not starting with `_`) and it's not imported

Griffe gained this method later on, to be used by mkdocstrings-python.

---

"Exported" is usually used to say "public" (we export an object to make it public), but in Griffe it's rather used to say "the object is exposed in regard to the wildcard import runtime mechanism". So we renamed "exported" to "wildcard exposed".

Then we fixed the issue mentioned above about private members being considered implicitly exposed. And since wildcard imports are not configurable, we dropped the distinction between explicitly and implicitly exposed. An object is either exposed, or it's not.

Finally the code that checks breaking changes now uses the "public" API instead of the "exposed" one.

Issue-281: #281
@pawamoy pawamoy closed this as completed Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant