Feature/cutsky factory#279
Conversation
| cache_key = (self.ngal, self._transform_state) | ||
| if cache_key in self._fsky_cache: | ||
| return self._fsky_cache[cache_key] |
There was a problem hiding this comment.
We could use native cache properties instead ?
There was a problem hiding this comment.
I didn't write this code, so I'm less familiar with what it's doing. My understanding is that ngal and _transform_state both return mutable values, so would we need to define __eq__ and __hash__ for BaseGalaxyCatalog and use @lru_cache for the two methods?
| pos_min, pos_max = self.get_reference_borders( | ||
| zranges, | ||
| boxsize, | ||
| region=region, | ||
| release=release, | ||
| program = program, | ||
| tracer = mask_tracer, | ||
| custom_healpix_mask=custom_healpix_mask, | ||
| ) | ||
| shifts = self.get_box_shifts(pos_min, pos_max, boxsize) | ||
| # doesn't make sense to have snapshotcatalog as input (multitracer) while having dataframe as output (all tracers forced into single dataframe) | ||
| replications = self.get_box_replications( | ||
| snapshot_catalog, | ||
| pos_min, | ||
| pos_max, | ||
| shifts=shifts, | ||
| distance_limits = (distance_limits[0], distance_limits[1]), | ||
| ) |
There was a problem hiding this comment.
Question 1: could those be used as external helpers or staticmethods ? (rule of thumb is that if you don't need self in the method, it should be OK to move it as a static method or an external function)
Question 2: could we move the tiling functions to SnapshotCatalog (either as staticmethods or helpers ?)
There was a problem hiding this comment.
self is used in both of these methods though, or maybe I'm misunderstanding the request. What did you mean?
I lean towards keeping the tiling in cutsky since it is really part of defining the cutsky geometry, but I could be convinced otherwise (since you're the one developing the snapshot classes, I'll let you implement that if you like)
2 participants
Draft PR for comments