Skip to content

Use jaraco.classes for properties - #588

Merged
jaraco merged 2 commits into
mainfrom
feature/jaraco.classes
Sep 1, 2022
Merged

jaraco merged 2 commits into
mainfrom
feature/jaraco.classes

Conversation

@jaraco

@jaraco jaraco commented Aug 8, 2022 •

Copy link
Copy Markdown
Owner

Today I was reviewing the implementation of NonDataProperty and ClassProperty only to find that there were stale versions of the implementations in keyring.util.properties that represent old versions of the original implementations in jaraco.classes.

I'm tempted to simply depend on that code. Because it introduces a new dependency, I'm somewhat reluctant to accept the change. I'm putting it here for feedback and consideration.

@Darsstar

Darsstar commented Aug 9, 2022

Copy link
Copy Markdown

Maybe reexport them for the time being for backward compatibility? (I'm tempted to replace "Maybe" with "At least", but I feel that makes me sounds too entitled.)

@jaraco

jaraco commented Sep 1, 2022

Copy link
Copy Markdown
Owner Author

Maybe reexport them for the time being for backward compatibility? (I'm tempted to replace "Maybe" with "At least", but I feel that makes me sounds too entitled.)

I can do that - make it an optional dependency for the time being.

@jaraco
jaraco force-pushed the feature/jaraco.classes branch from e110fb2 to 3c38fa4 Compare September 1, 2022 00:32
@jaraco
jaraco merged commit 977ed03 into main Sep 1, 2022
@jaraco
jaraco deleted the feature/jaraco.classes branch September 1, 2022 12:33
@Thaodan

Thaodan commented Sep 8, 2022

Copy link
Copy Markdown

Isn't that kinda much to add another dependency just for these two methods?

@jaraco

jaraco commented Sep 8, 2022

Copy link
Copy Markdown
Owner Author

Yes and no.

  1. In an ideal world, dependencies are free. And for most users, that's also true (pip install keyring just works in all cases). Of course, there are practical, non-ideal considerations.
  2. There are several costs to limiting dependencies. It increases friction when creating new functionality. It encourages vendoring (maintaining a local fork) of the functionality, which has its own maintenance costs and risks. It discourages best practices around dependency management.
  3. Not using the dependency masks the true dependency. It hides the fact that this functionality is useful across multiple projects, making it nearly impossible to determine if this shared need would be valuable to be exposed elsewhere (stdlib, for example).
  4. This case is a perfect case for including the dependency. There was an old, stale implementation that was copied from many years ago. Since then, the implementation has improved and evolved. In the meantime, other libraries developed a dependency on this vendored fork of the code.

Principles I roughly follow for dependencies:

  • If a small (~10 lines or less) piece of code is useful, it's okay to copy it, but still link to the source from which it was copied.
  • Consider using a dependency when the code is larger, there is more than one function, or the behavior is likely to evolve.
  • Take extra precautions when adopting the first dependency, as the distance from 0 to 1 is much greater than adding one more.

I think implicit in your question is that adding a dependency is a cost that should be taken seriously. Can you articulate or link to a resource that explains those costs?

@yan12125

Copy link
Copy Markdown
try:
    from jaraco.compat import properties  # pragma: no-cover
except ImportError:
    from . import _properties_compat as properties  # pragma: no-cover

I think from jaraco.compat import properties should be from jaraco.classes import properties?

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.

4 participants