search

rs-station / rs-booster

Useful scripts for analyzing diffraction
https://rs-station.github.io/rs-booster/
MIT License
3 stars 4 forks source link

integration of low occupancy map scripts #29

Open alisiafadini opened 2 years ago

alisiafadini commented 2 years ago

I have the scripts we talked about (background subtraction, TV denoising, other utils) in my own repository, which is where I started developing things before we spoke...

I tried to give it a space-related name – METEOR for now but COMET (Crystal Occupancy Map Enhancement Tools) also an option, or open to brainstorming. Some parts of scripts are still a bit of a work in progress.

It essentially contains meteor.py with all the functions I use across scripts, and then 3 scripts for actually generating new files. I am not sure what the best way to integrate this in rs-booster would be. Probably need to make changes to where all the functions are stated. Any thoughts?

I was thinking it could be its own subfolder (like scaling, maps, meteor...) so it remains a separate low-occupancy species set of map tools. Let me know what you think! All of this open to what you think makes the most sense

JBGreisman commented 2 years ago

I'm a fan of METEOR -- the phrase "ephemeral occupancy" feels right, too. Much more evocative than "partial occupancy" or "low occupancy"...

I'm currently in the midst of a small refactor to move around subfolders (per #24). I think having it as its own submodule may be a good solution to avoid any conflicts, and to also provide any useful sub-functions via its own submodule in the Python API -- i.e.:

from rsbooster.meteor import foo,  bar

As far as the commandline interface, I'll need to have a bit more of a look at your repo to understand what's going on. This may be the first workflow in rsbooster that requires multiple calls to accomplish a task, so we should think a bit about how to best do this cleanly. My initial thought would be to use a single command (let's say rs.meteor) and subparsers for each step: rs.meteor tv_filter, rs.meteor subtract_background... etc. Please let me know your thoughts

alisiafadini commented 2 years ago

Glad ephemeral gets at least one vote :alien:

I agree that is sounds like having it as its own submodule: rsbooster.meteor would work – straightforward.

For potential subparsers and commandline aspect, I think you may have a better idea as to what makes sense. Not sure if @tjlane has an opinion too. Definitely let me know if I can rearrange/modify anything in my repo to help the process.

alisiafadini commented 1 year ago

@JBGreisman any ideas on this? Not sure if you've had any time to check out the repo. let me know if I can do anything!

JBGreisman commented 1 year ago

Sorry -- hadn't realized something here was waiting on my feedback. I'll have a look through your repo and will get back to you with suggestions for setting up a commandline interface

alisiafadini commented 1 year ago

No worries and no rush! I am also just getting back to working on it more intensively now

dennisbrookner commented 1 year ago

Hey Alisia! (and Jack!) I just noticed this issue, and I figured I would chime in with the alternative option of wrapping up your code as its own python package and adding it to the Reciprocal Space Station family. You'd get a card on the homepage here: https://rs-station.github.io/ and could set up docs at rs-station.github.io/meteor, but keep autonomy over the API and such.

JBGreisman commented 1 year ago

I agree with Dennis' comment -- I think that would be a good way to support that code if you're still interested in hosting it here.

alisiafadini commented 1 year ago

Hey guys,

absolutely! I got a little side-tracked with starting a new job but I was actually hoping to wrap everything up and come out with a preprint sometime in September/October.

Your suggestion Dennis sounds good to me

What would be the next steps?

From: Jack Greisman @.> Sent: Friday, September 1, 2023 3:13 AM To: rs-station/rs-booster @.> Cc: Alisia Fadini @.>; Author @.> Subject: Re: [rs-station/rs-booster] integration of low occupancy map scripts (Issue #29)

I agree with Dennis' comment -- I think that would be a good way to support that code if you're still interested in hosting it here.

— Reply to this email directly, view it on GitHubhttps://github.com/rs-station/rs-booster/issues/29#issuecomment-1702016473, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AT6GFSDWKVXKFZC22F6KTUDXYFADVANCNFSM6AAAAAAQ4PXGEE. You are receiving this because you authored the thread.Message ID: @.***>

dennisbrookner commented 1 year ago

I think that you can go ahead and transfer the repo over to the rs-station org! You should have the permissions to do so.

The next step that I'd say is essential is getting the package on PyPI. Tragically meteor is taken; you could potentially get the name by wrestling it from the current person squatting there (https://pypi.org/project/meteor/) or you could go with meteorx or something similar as the official package name.

The other thing that I'd prefer to happen at some point would be to build online documentation, similar to https://rs-station.github.io/rs-booster/index.html. Nothing fancy, basically just a landing page and a transcription of the function docstrings. I can help you get that all set up if that's useful!

alisiafadini commented 1 year ago

Crushing that meteor is taken on PyPI – seems likes it's used by a few projects so maybe better to find an alternative. rs-meteor could be an option?

I can start working on transferring to rs-station and the documentation in the coming days!

tjlane commented 3 weeks ago

I'd just like to vote we close this, seeing as we went with @dennisbrookner 's suggestion: https://github.com/rs-station/meteor