pyOpenSci / software-submission

Submit your package for review by pyOpenSci here! If you have questions please post them here: https://pyopensci.discourse.group/
94 stars 36 forks source link

Presubmission inquiry for plenoptic #97

Closed billbrod closed 1 year ago

billbrod commented 1 year ago

Submitting Author: @billbrod

Package Name: plenoptic

One-Line Description of Package: a python library for model-based image synthesis.

Repository Link (if existing): https://github.com/LabForComputationalVision/plenoptic/ (this PR docs build may be more useful for an introduction, still haven't merged into main yet)


Code of Conduct & Commitment to Maintain Package

Description

Community Partnerships

We partner with communities to support peer review with an additional layer of checks that satisfy community requirements. If your package fits into an existing community please check below:

Scope

Domain Specific & Community Partnerships

- [ ] Geospatial
- [ ] Education
- [ ] Pangeo
- [X] Unsure/Other (explain below)

I think plenoptic is actually out of scope, but I wanted to check, because pyopensci looks cool. This package is intended for use by the vision science, machine learning, and neuroscience communities, but could be used by any researcher that builds models that take something image-, video-, or audio-like as input. The package generates new stimuli (for use in further experiments) rather than facilitates the visualization of existing data.

Researchers in vision science, machine learning, and neuroscience, largely. The goal is to generate novel stimuli (images, videos, audio) that researchers can use in new experiments to better understand their computational models.

Not that I'm aware of.

P.S. Have feedback/comments about our review process? Leave a comment here

NickleDave commented 1 year ago

Welcome @billbrod!
Thank you for raising a presubmission issue.

The short answer is that, yes, plenoptic is in scope.

I can understand why that might not have been clear: you were looking for a category in our guide that the package fits into?

It definitely falls under the category of "high-level package" in the sense described at the top of the page here: https://www.pyopensci.org/software-peer-review/about/package-scope.html#what-types-of-packages-does-pyopensci-review

As you noted

this package is intended for use by the vision science, machine learning, and neuroscience communities

and so reviewing the package would help us help you support scientific workflows in those domains. I am a little familiar with this area, so I feel safe saying that I think I part of what's going on here is that methods are still evolving. But making it easier for other people to apply and explore these methods will facilitate that evolution. Open science ✨😼 And of course it's line with everything you (all) are doing at Flatiron.

Along the same lines: some feedback from our editorial team was that the purpose of the project was not clear. You made it clear to me here at least, but I agree that "surfacing" the core functionality on the index page and README would really help. It looks like the PR docs build you linked to will go a long way towards helping with that. I'd suggest adding similar images to the README if possible. But this is just tweaking the delivery; all the right info is there.

It looks like you are a good way towards meeting all the requirements. One thing I noticed is that you are using a setup.py. It's not quite clear to me at first glance if you would be able to instead use a modern pyproject.toml with declarative metadata. I'd strongly suggest doing so if you can. I'm also a little surprised to see some of the modules listed as packages in the setup.py--not sure if that's necessary? But maybe there's some complexity to the build I'm missing. Just some quick feedback I noticed on an initial look-through.

In short: please proceed with a full submission if you would like, and we will be happy to provide you with a review. We can close this once you open that submission. I'm also happy to answer more questions here.

NickleDave commented 1 year ago

One more comment: as far as related packages you might consider including links to the texforms work from @brialorelle @ArturoDeza if you don't have them in the docs already, e.g. https://github.com/brialorelle/texformgen and https://github.com/ArturoDeza/Fast-Texforms (that IIUC are an extension of metamers)

billbrod commented 1 year ago

Thanks for your response!

Yes, I thought that since it's a "high level package", it had to fall within one of the supported domain areas. That's not the case right now? Then there is no domain restriction?

But I'm glad to hear plenoptic is in scope! Yes, the purpose definitely needs to be more clear. I've got some open PRs to finish and merge first (including the docs one I linked and another about updating the README), but then I'll proceed with a full submission, probably around the time of our presentation at VSS.

To answer your comments:

  1. I'm unclear about the distinctions between the different ways of packaging python packages, and setup.py was just the one I was already familiar with. I did just come across your packaging guideline recently, and am planning on reading it over. If pyproject.toml is strongly recommended, I can switch to that.
  2. I don't think the modules listed under packages in setup.py are necessary: again, legacy of what I did before (they may have been necessary in pyrtools, the first library I packaged, which has some C code). Do you have a good explainer for what listed them under packages does? Again, it's unclear to me.
  3. For related packages, I'm unclear how much they have to be "packages" rather than "research artifacts", for lack of a better term. There are plenty of related github repos of code that are associated with research papers (including those), but in my mind they're not packages, because they're meant to reproduce a specific paper, rather than work generally, are maintained and pip-installable, etc. Is that not a useful distinction? I agree we should definitely link them as related work somewhere in the docs, though.
NickleDave commented 1 year ago

Then there is no domain restriction?

"High level package" is sort of the top-level criteria; there's no restriction on research domain (e.g., neuroscience). The categories guide what we focus on in terms of open science but there is no requirement that a package fit neatly into only one of those categories.

I'll proceed with a full submission, probably around the time of our presentation at VSS.

Perfect. By the way, in case you didn't see / weren't aware, we partner with JOSS

Do you have a good explainer for what listed them under packages does?

I think the point of being able to list multiple things in packages is when you have a "monorepo" type situation as shown in this SO post: https://stackoverflow.com/questions/42859852/distributing-multiple-packages-using-setup-py-for-python In your case it works because a single module can technically be a package, I think. But that is a guess. I would try installing + building with pyproject.toml, and if that works, avoid worrying about setuptools.setup unless you need to for a package like pyrtools.

Is that not a useful distinction?

Yes, sorry I should have checked whether the Python project was a full-fledged package. If not then I agree that citing in a "Related works" page of the docs is appropriate here.

Look forward to your submission, thank you!

billbrod commented 1 year ago

"High level package" is sort of the top-level criteria; there's no restriction on research domain (e.g., neuroscience). The categories guide what we focus on in terms of open science but there is no requirement that a package fit neatly into only one of those categories.

Ah okay, I misunderstood. Glad I opened this presubmission inquiry!

By the way, in case you didn't see / weren't aware, we partner with JOSS

Yup! I'm happy about that.

Thanks for your clarifications / answers and I'll submit this soon.

Should we close this issue now or after the submission?

NickleDave commented 1 year ago

Glad I opened this presubmission inquiry!

Us too 🙂 How we describe scope is still evolving as we get back into gear--if you have feedback on how we could make it clearer that in general we're happy to review higher level packages, happy to hear it

Should we close this issue now or after the submission? We can close now but please reference this issue (by issue # I mean) when you open the submission

billbrod commented 1 year ago

I was confused about scope since they say: "pyOpenSci reviews packages that fall within a list of specified categories and domains. Packages must also meet our technical scope requirements." So I then looked through the Package categories and Domain areas lists, and assumed that, if my package didn't fall under one of those categories, it was not in scope. To change it maybe change that line I quoted to say "pyOpenSci reviews packages that fall within a list of specified categories and domains, as well as high-level packages in other areas. Packages must also meet our technical scope requirements." or something similar? Or, at the top of Domain areas, add a note along the lines of "high-level scientific packages in other domains are also encouraged to submit, but these domain areas are particularly well-supported / we have existing ties / something" (I'm not clear on what's different about the domains listed on the website, to be honest).

We can close now but please reference this issue (by issue # I mean) when you open the submission

Will do!