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

Implement array trait, drop traittypes dependency #161

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

chrishavlin
Copy link
Contributor

traittypes is unmaintaned (last code modification 4 years ago) and is the source of a pytest warning we've been getting for a while:

../../../../../opt/hostedtoolcache/Python/3.9.20/x64/lib/python3.9/site-packages/traitlets/__init__.py:28
  /opt/hostedtoolcache/Python/3.9.20/x64/lib/python3.9/site-packages/traitlets/__init__.py:28: DeprecationWarning: 
              Sentinel is not a public part of the traitlets API.
              It was published by mistake, and may be removed in the future.

This is a minimal replacement for traittypes.Array that I think covers what we need. The main thing I did not add is any explicit dtype handling -- I could, but yt_idv isn't using any at the moment so I didn't add it (but don't think it'd be hard)... I also could refactor YTPositionTrait to inherit from the new ArrayTrait, but thought it'd be better to make that as a subsequent change.

@chrishavlin chrishavlin added the infrastructure Related to CI, versioning, websites, organizational issues, etc label Dec 6, 2024
@chrishavlin chrishavlin added this to the 0.5.0 milestone Dec 6, 2024
@matthewturk
Copy link
Member

Can we also try submitting a PR to traittypes?

@chrishavlin
Copy link
Contributor Author

just so i don't forget and accidentally merge this, gonna switch to draft til we decide what to do re: upstream PRs. This PR is a pretty low priority -- it's unlikely that traitlets will actually deprecate the Sentinel class anytime soon.

@chrishavlin chrishavlin marked this pull request as draft December 10, 2024 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Related to CI, versioning, websites, organizational issues, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants