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

v0.7 #171

Closed
7 of 8 tasks
ivirshup opened this issue Jun 27, 2019 · 29 comments
Closed
7 of 8 tasks

v0.7 #171

ivirshup opened this issue Jun 27, 2019 · 29 comments
Milestone

Comments

@ivirshup
Copy link
Member

ivirshup commented Jun 27, 2019

I'd like to have some organization around the upcoming release. I'd like to have a list of all planned and included changes for v0.7 Please edit this for anything I forgot!

New features

Breaking changes

TODO:

@flying-sheep
Copy link
Member

Do we want “2d only” (the second breaking change) in something called 0.7?

I think to signal a breaking change and closeness to 1.0, we could name this version 0.90 or so!

(not 0.99, because then we’ll end up having 0.999, 0.9999, …)

@flying-sheep
Copy link
Member

Another change we could think about: unifying .layers and .X

Maybe .X should just be a property returning the first layer? or the layer called X?

@ivirshup
Copy link
Member Author

Do we want “2d only” (the second breaking change) in something called 0.7?

I think to signal a breaking change and closeness to 1.0, we could name this version 0.90 or so!

I would understand the change from v0.6.x to v0.7 to involve breaking changes. I don't like the idea of going to v0.90 since it breaks from semantic versioning. Also, who knows, we may want v0.8-v0.20. For example, I'd say the possibility of having .X be one of .layers sounds like a great v0.8 target to me, since it would require us to allow layers to be backed, and that'll take some time.

I would have to know that this was a dramatic change that broke a lot of stuff before thinking any out of the ordinary versioning semantics were used. The change to always 2d doesn't break many tests here or for scanpy. Additionally scvelo mostly runs with it.

I do think we should do a v0.6.22 release of AnnData and v1.4.4 of scanpy, so there's a version with working scipy/ statsmodels dependencies without this change. This would also give us space to transition users to {obs,var}_vector, since that will return a 1d array now, and once "always 2d" is in effect.

@flying-sheep
Copy link
Member

Well, if the fallout isn’t that bad, I agree, we can just call it 0.7

That being said, skipping from 0.6.x to 0.90 is completely compatible with semantic versioning. Why do you think it wasn’t? (And instead of 0.8, the next version would just be called 0.91)

@falexwolf
Copy link
Member

@ivirshup: Agreed with everything! Terrific! Agreed in particular with 0.6.22 and 1.4.4 before moving to 1.5 and 0.7. In 0.6.22, where the flattening is still present, can you add a warning future versions will not flatten .X anymore when it's called?

@ivirshup
Copy link
Member Author

@flying-sheep, I suppose v0.90 would be compatible with establishing the order of versions, but I think of "incrementing the version" to imply incrementing on a fixed scale. I think my reaction to seeing a version change from v0.6.22 to v0.7.0 would be "there are breaking changes in this release", while v0.6.22 to v0.90.0 would be "what does this mean? What happened to v0.7-v0.89"?

@ivirshup
Copy link
Member Author

ivirshup commented Jul 4, 2019

I've added that warning, what else has to be done before we can release 0.6.22?

@falexwolf
Copy link
Member

Nothing, I think. I'm happy to make the release if you don't want to do it. 🙂

@ivirshup
Copy link
Member Author

ivirshup commented Jul 5, 2019

Just realized there are probably a couple bugs to fix prior to that:

@falexwolf
Copy link
Member

Great that you're addressing these!

I think it should always make a copy if its on a view.

Agreed!

@ivirshup
Copy link
Member Author

ivirshup commented Jul 6, 2019

I think we're good for 0.6.22. Scanpy will need a pass to remove warnings being thrown for v1.4.4 though. Would you mind making this release @falexwolf?

ivirshup added a commit to ivirshup/anndata that referenced this issue Jul 7, 2019
Previously had relied on subsetting the entire object to get vector of X. Now just normalizes index.
Also stops throwing warning about changing behaviour. Whoops. scverse#171 (comment)
ivirshup added a commit that referenced this issue Jul 8, 2019
Previously had relied on subsetting the entire object to get vector of X. Now just normalizes index.
Also stops throwing warning about changing behaviour. Whoops. #171 (comment)
@flying-sheep
Copy link
Member

flying-sheep commented Jul 10, 2019

It would be great if you could start publishing wheels too, it’s almost no additional work:

python3 setup.py sdist bdist_wheel
twine upload dist/anndata-0.7{.tar.gz,-py3-none-any.whl}

I always have to go back and retroactively make wheels.

@falexwolf
Copy link
Member

Oh sorry, I thought we stopped publishing wheels 1.5 years after going from cython to numba. 😆Had you told me that we wanted to continue (what for, btw?, there is no compilation involved), I'd continued publishing wheels of Mac.

@flying-sheep
Copy link
Member

There’s not that much of an advantage anymore but wheels are faster to install, as setup.py doesn’t need to be run on the target machines – the wheels will simply be unpacked.

@ivirshup
Copy link
Member Author

Could the release process get documented, potentially with scripts or snippets? I'd like to be able to help out with that, but I'd like to know I was doing it right.

@flying-sheep
Copy link
Member

It’s not more than this, but we can document it, yes:

$ git tag x.y.z
$ python3 setup.py sdist bdist_wheel
$ twine upload scanpy-x.y.z-py3-none-any.whl scanpy-x.y.z.tar.gz
$ git push --tags

@ivirshup ivirshup pinned this issue Jul 29, 2019
@ivirshup
Copy link
Member Author

ivirshup commented Sep 6, 2019

Just push a big update which cleans up the reading and writing code a lot, as well as improving performance. It does make some changes to the on disk structure, but you should be able to read older files. Added features mean you can now have data frames and sparse matrices stored in any of the obsm, varm, layers etc.

It would be great if people could try out current master and let me know if they find any bugs!

@flying-sheep
Copy link
Member

OK, so should we get this out before the X-layers merge (#244) and the modes (#237) land?

@ivirshup
Copy link
Member Author

ivirshup commented Nov 7, 2019

Yes! I need to take another look at fixing scverse/scanpy#832 before this can go though. I'm going to check if there's any chance this could get sorted upstream first.

@flying-sheep
Copy link
Member

Hmm, we should deprecate rename_categories, as it’s no longer necessary with the new categorical storage. I’ll do this before releasing the rc.

@LuckyMD
Copy link

LuckyMD commented Dec 22, 2019

What is the alternative to rename_categories? I'm using it quite a lot. It works both on the categoricals for the obs or var column, but also on any rank_genes_groups() results that are generated, no?

@flying-sheep
Copy link
Member

flying-sheep commented Dec 23, 2019

For .obs/.var it’s superfluous, just do:

adata.obs['foo'].cat.categories = new_categories
# or
adata.obs['foo'].cat.rename_categories(..., inplace=True)

For the .uns stuff, we need to find another way, this is a scanpy convention and doesn’t belong into AnnData. Either move it to scanpy or store it in a way that allows to do .uns[...].rename_categories(...)

@LuckyMD
Copy link

LuckyMD commented Dec 23, 2019

The nice thing about rename_categories as it is at the moment is that it renames the .uns and .obs/.var stuff in a single command.

@flying-sheep
Copy link
Member

We just (pre-)released AnnData 0.7rc1 with it deprecated. We should move the code to scanpy as a documented utility I think.

@flying-sheep
Copy link
Member

flying-sheep commented Dec 30, 2019

Alex reverted stuff on master because there’s no alternatives, so my idea of our path forwards is now:

  1. Scanpy needs to grow utils.rename_categories (or so) and update_anndata (or so)
  2. obsp/varp warning and rename_categories deprecations needs to be reinstated before 0.7
  3. people should be encouraged at the right places to migrate to obsp/varp (using update_anndata) and scanpy.utils.rename_categories.

@falexwolf
Copy link
Member

Could we try to have a discussion instead of you making absolute statements?

  1. semi-agreed, because of unclarity of alternatives, see our Slack discussion
  2. disagreed on obsp/varp, it gives the user a horrible experience, and there are no alternatives, I worked with these warnings for a week now and got very annoyed, so I muted them
  3. agreed

@flying-sheep
Copy link
Member

flying-sheep commented Dec 30, 2019

Sorry for not outlining this clearly enough, I was busy celebrating Christmas and my birthday!

OK, so we have two changes that need to happen (at least in the long, if not short run). We should only have the deprecations in the final anndata 0.7 release if we manage to get the correct behavior into anndata and scanpy quickly enough.

  1. Better categorical handling: You think instead of deprecating rename_categories, we should allow storing arrays with a categorical dtype in .uns, right? You said in Slack that this is a difficult change. Can you elaborate why?
  2. obsp/varp instead of square matrices in .uns: I don’t think this is a difficult change: We just need to figure out all places where scanpy currently stores such a matrix and change both the code and add that storage position to a list of “to be modernized” AnnData parts that can be used implicitly or via update_anndata.

I’d like 2 to happen before the next release because obsp/varp exists already and we need to keep people from relying on the old behavior.

@flying-sheep
Copy link
Member

OK, seems like we got it. Now to all the exciting changes enabled by this!

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

No branches or pull requests

4 participants