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

cooler >0.9.0 support #150

Closed
sergpolly opened this issue Apr 20, 2023 · 7 comments · Fixed by #151
Closed

cooler >0.9.0 support #150

sergpolly opened this issue Apr 20, 2023 · 7 comments · Fixed by #151

Comments

@sergpolly
Copy link

there is a little conundrum in the higlass-python/clodius/cooler/cooltoos interoperability now:

  • python-higlass is great for interactive exploration in jupyter, it requires clodius for serving local coolers
  • clodius currently requires cooler<0.9.0
  • cooltools requires cooler>=0.9.1 - thus cannot be used together with higlass-jupyter/clodius in the same jupyter notebook (such a bummer !)

what is incompatible between cooler 0.9.1 and clodius at the moment ? is it just tiles/cooler.py ?
I'm willing to look at it - just trying to narrow down the issue ...

@sergpolly
Copy link
Author

anecdotally - simply removing max_chunk=np.inf argument from clr.matrix() call:

matrix = c.matrix(as_pixels=True, balance=False, max_chunk=np.inf)
fixes tests in tests/tiles/cooler.py

todo - test higlass-python with such modified clodius for serving local coolers

@manzt
Copy link
Member

manzt commented Apr 20, 2023

I know @nvictus was prototyping a new tile fetcher for clodius > 0.9. Maybe that’s in a state he can share?

@sergpolly
Copy link
Author

hm - so far I can confirm that, the simple fix was enough ... and higlass-python is happily serving me a local cooler from within open2c examples :
Screenshot from 2023-04-20 12-30-16

@manzt
Copy link
Member

manzt commented Apr 20, 2023

It's not clear to me what this used to do because the max_chunk parameter isn't included in the old docstring.

https://github.com/open2c/cooler/blob/ac779db1899a1ba295a0a47db3de5873aaa47ac1/cooler/api.py#L301-L341

I guess we could either choose to remove the parameter or have the newer cooler API accept **kwargs as a final catch all for kwargs. My feeling is to just remove max_chunk as you have, and pin cooler>=0.9.0 in the pyproject.toml for clodius.

@sergpolly
Copy link
Author

here is what I found in the old version:
Screenshot from 2023-04-20 19-40-27
CSRReader is where matrix method uses max_chunk parameter - so I think it would be fair to say that it used to control performance/memory-footprint tradeoff ...

It is unclear however what happened to this "contol knob" in the 0.9.0...
I know that 0.9.0 was a major refactoring of the query engine...
Wait... - the new API (e.g. matrix method) has chunksize (!) which remains undocumented, - perhaps it does the similar thing as max_chunk before

@nvictus
Copy link
Member

nvictus commented Apr 21, 2023

Yes, removing max_chunk should do the trick until the new tile fetcher is merged.

@Phlya
Copy link

Phlya commented May 12, 2023

Just wanted to upvote this issue, especially since the fix appears to be simple.

nvictus added a commit that referenced this issue May 15, 2023
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 a pull request may close this issue.

4 participants