S7_class becomes a reserved attribute#677
Closed
lawremi wants to merge 1 commit into
Closed
Conversation
Member
|
Maybe do after #673? |
Member
|
Or maybe we should consider renaming this to |
Collaborator
Author
|
No strong feelings, but if we're going to rename it, the sooner the better, to minimize the breakage of serialized instances. |
This was referenced Jun 8, 2026
t-kalinowski
added a commit
that referenced
this pull request
Jun 23, 2026
* Address external class review findings
* Fix deferred external class method unregistration
* Resolve external class signatures on unload
* Fix external class validation and cleanup
* Fix external-class method hook cleanup
* Fix external class dispatch handling
* Fix deferred external class registration
* Resolve external signatures on method unregister
* Check external class versions before string match
* Remove stale S7 hooks after failed loads
* Reject package-less external class matches
* Normalize external generic sentinels
* Register runtime external-class methods
* Fix deferred external class method registration
* Fix external generic hook replacement
* Short-circuit union class checks
* Address external method registration reviews
* Fix external class union handling
* Remove stale deferred external methods
* Fix external method unregister review cases
* Fix deferred union registration
* Preserve versioned external generic on unregister
* Allow external classes to narrow S7_object
* Simplify external class matching
* Simplify external class cleanup
* Simplify external class registration
* Trim external class changes
* Trim external class cleanup
* Simplify external class resolution
* Simplify external dependency checks
* Use closure for external class validation
* Remove external S7_object special case
* Document generic sentinel invariant
* Trim-external-class-tests
* Clarify-external-package-vignette
* Clarify external class resolution errors
* Restore external class docs example
* Restore external dependency flattening helper
* Simplify external class error construction
* Snapshot external class binding mismatches
* Restore external class default snapshot
* Reduce external class snapshot churn
* Fix external class review findings
[P2] Allow subclasses of external parent classes — /Users/tomasz/github/RConsortium/S7/R/class-spec.R:372-375: Valid narrowing is rejected when a child class is a real S7 subclass of an external parent class. The branch returns FALSE unless the child has the exact same package/name as the external placeholder, so cases like dep::Sub extending dep::Base cannot override a parent property typed as new_external_class("dep", "Base") even though the inheritance chain would make it valid.
[P2] Remove methods when an external class unloads first — /Users/tomasz/github/RConsortium/S7/R/hooks.R:89-90: If the external class package unloads before the downstream package, external_deps_resolvable(deps) is false and unload uses the placeholder signature. Since the method was registered with resolved class objects, unregister_own_S7_method() builds a non-identical method and leaves the downstream method installed in the upstream generic after S7_on_unload().
* Rename `S7_class` attribute to `_S7_class` (#689)
This keeps the name as similar as possible, but makes it clear that it's in the namespace S7 reserves for its own usage. Includes backward compatibility fallback that we can remove in a future version.
Closes #677
* Clarify external class resolution errors
* avoid binding `getNamespaceVersion` in package namespace
* Refactor test helpers to use code blocks (#714)
---------
Co-authored-by: Hadley Wickham <h.wickham@gmail.com>
kosmika
pushed a commit
to kosmika/RConsortium_S7
that referenced
this pull request
Jun 24, 2026
This keeps the name as similar as possible, but makes it clear that it's in the namespace S7 reserves for its own usage. Includes backward compatibility fallback that we can remove in a future version. Closes RConsortium#677
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is hopefully an obvious change where the
S7_classattribute is no longer allowed to be declared as a property. Just noticed this while I was working on the S4 support.