search

ryanhausen / fitsmap

FitsMap: A Simple, Lightweight Tool For Displaying Interactive Astronomical Image and Catalog Data
MIT License
46 stars 9 forks source link

Expose the min_points argument of supercluster #85

Closed lmytime closed 6 months ago

lmytime commented 6 months ago

min_points argument is important to change the catalog point clustering parameter or even set it to a large number to avoid catalog clustering.

This tiny change can allow one to create a catalog layer without point clustering when setting cluster_min_points=99999999 :)

ryanhausen commented 6 months ago

Thanks for the contribution! This is a great idea and something I have been meaning to get to.

Do you think there is value in being able to modify the other arguments to supercluster?

lmytime commented 6 months ago

Hi @ryanhausen , I think exposing more arguments of supercluster could be helpful for some customizing purposes. Someone would like it.

ryanhausen commented 6 months ago

Are you interested in making these changes? I would think that radius and node_size might be helpful. I am not sure tweaking the other ones would be super useful.

If they all have prefix cluster with a doc string description, like you have already done for min_points, it should be clear to other users. If you set the default values to the values I use right now all the tests should pass without modification as well.

Let me know what you think.

lmytime commented 6 months ago

That is a good idea. I'm willing to do these things.

lmytime commented 6 months ago

Thank you very much! I have fixed the name error and mirrored new arguments. Please let me know if any new errors.

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (6ff707c) 90.41% compared to head (5642e0c) 90.46%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #85 +/- ## ========================================== + Coverage 90.41% 90.46% +0.05% ========================================== Files 5 5 Lines 741 745 +4 ========================================== + Hits 670 674 +4 Misses 71 71 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ryanhausen commented 6 months ago

@lmytime Thanks the tests pass which is great. The last thing and then we're all done is that though you added the arguments to dir_to_map but the arguments aren't passed to the files_to_map call at the bottom of dir_to_map. Add those, I'll rerun the tests and we'll merge.

Thanks!

lmytime commented 6 months ago

@ryanhausen Thanks for checking! Done.

ryanhausen commented 6 months ago

LTGM! Thanks again @lmytime. I will push the changes to pypi probably over the weekend.