-
Notifications
You must be signed in to change notification settings - Fork 157
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
Serialize Nones #999
base: main
Are you sure you want to change the base?
Serialize Nones #999
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #999 +/- ##
==========================================
- Coverage 87.08% 84.63% -2.45%
==========================================
Files 40 40
Lines 6107 6116 +9
==========================================
- Hits 5318 5176 -142
- Misses 789 940 +151
|
As discussed, I don't want to change the on-disk format in the next release. So I'd hold off on this until at least 0.11. |
Since 0.10 is released, we can document and then merge this, right? |
I would like to have proposals for all disk format changes written up and discussed with other implementers before merging them to main. |
Sure, we can use this PR as reference implementation. Which other implementers are there? Is there a channel with the relevant people on some communication medium? |
Hi there, I feel that this is an important feature/bug fix, I was wondering if there were plans to merge this anytime soon/if it's possible to help. |
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.
Two things:
- Should we put this feature behind a flag initially? I know we had spoken about that as a way forward before. However, in this case, I'm no really sure what we'd gain by doing that.
- This PR is simple enough that it can be merged without much comment. I think we should try to make it clear that this PR is planned for the 0.12 release and that we are looking for feedback (specifically from other languages).
I’m not super hot on the idea myself. The whole idea behind IOSpecs was that people get a nice error message that says: “this file has been written by a newer anndata version, so use that to load it”. Now we have both that and individual feature flags for e.g. nullable arrays. The intended purpose for the latter is that people see the feature but have to opt-in. But then again they already have to (less explicitly) opt in by using Now that I’m thinking about it, an alternative possible solution could have been compatibility profiles for each AnnData version with new features:
I don’t understand the first half: With it being milestone 0.12, we don’t backport its changelog entry to the 0.11.x branch, that’s what makes this change 0.12. What kind of feedback are we looking for? |
More just going off of #673 (comment). Maybe we'll learn something but honestly, from what I can tell, this feature should not be a problem for the programming languages that has anndata support (from memory).
Ok, I think since neither of us can come up with an idea, then maybe we just merge this PR and note to the community that this feature is coming, and if they want to try it out, it's on |
.write
does not saveNone
values #673Rendered release notes