astropy / pyregion

ds9 region parser for python
https://pyregion.readthedocs.io
MIT License
39 stars 40 forks source link

Private functions shouldn't be promoted to be used #129

Closed bsipocz closed 6 years ago

bsipocz commented 6 years ago

While I'm hunting for some weird CI issues in astroquery, I've run into this in pyregion. IMO we shouldn'y promote usage of private functions in the getting started page, if this needs to be in the API then expose it properly in the API docs, etc.

>>> import pyregion._region_filter as filter

cdeil commented 6 years ago

@leejjoon - Brigitta is referring to this example that you added 9 years ago: https://github.com/astropy/pyregion/commit/807acbd9a94cd72f31e2213698ad5885c7aadb36#diff-34754436d3d51c72ac3cfcec1be6f3ecR181

>>> import pyregion._region_filter as filter
>>> myfilter = filter.Circle(0, 0, 10) & filter.Box(15, 0, 10, 10)
>>> myfilter.inside1(0, 0)
0
>>> myfilter.inside1(10, 0)
1
>>> myfilter.inside([0, 10], [0, 0])
array([False,  True], dtype=bool)

The Circle and Box aren't listed in the public API docs, and my suggestion would be to simply delete that example.

The alternative is to properly expose that part of pyregion, and to spend time documenting it, which IMO doesn't make much sense given that by now there's http://astropy-regions.readthedocs.io/ which is build on Astropy, has region classes and mask creating implemented.

@leejjoon - Thoughts?

leejjoon commented 6 years ago

Yes, it seems that it is best to just delete those examples.