-
Notifications
You must be signed in to change notification settings - Fork 25
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
[python] Support enumerations in update_obs
/update_var
#1707
Conversation
2db25ea
to
5fdd9a3
Compare
update_obs
/update_var
[WIP]update_obs
/update_var
[WIP]
5fdd9a3
to
07133e9
Compare
07133e9
to
f636d09
Compare
Codecov ReportAll modified lines are covered by tests ✅ ❗ Your organization needs to install the Codecov GitHub app to enable full functionality. see 37 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
update_obs
/update_var
[WIP]update_obs
/update_var
[WIP]
This is having a weird CI fail cc @ihnorton I thought we resolved all mac-CI issues a couple weeks ago but I may have missed something ... 👀 🤔 |
update_obs
/update_var
[WIP]update_obs
/update_var
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple minor code style points but otherwise looks mostly good. Not familiar enough with the way enumerations work to judge that part but provided somebody can OK that, I'm happy.
* [python] Support enumerations in `update_obs`/`update_var` [WIP] * expand unit tests * silence a warning * Avoid unnecessary MacOS 3.7 check * experimenting * code-review feedback
…1736) * [python] Support enumerations in `update_obs`/`update_var` [WIP] * expand unit tests * silence a warning * Avoid unnecessary MacOS 3.7 check * experimenting * code-review feedback Co-authored-by: John Kerl <[email protected]>
Issue and/or context: Validates
update_obs
/update_var
when the newly added column is an enumeration: tracking issue #1710, with grandparent #866.Changes:
Notes for Reviewer: