Fluidimage submission #194

paugier · 2024-05-30T12:53:48Z

Submitting Author: Pierre Augier (@paugier)
All current maintainers: (@paugier)
Package Name: Fluidimage
One-Line Description of Package: "Fluidimage: a Python framework to study flows from images"
Repository Link: https://foss.heptapod.net/fluiddyn/fluidimage
Version submitted: 0.5.3
EIC: @Batalex
Editor: @cmarmo
Reviewer 1: @leahmendelson
Reviewer 2: @stefanv
Reviewer 3: @emmylia321
Archive: TBD
JOSS DOI: TBD
Version accepted: TBD
Date accepted (month/day/year): TBD

Code of Conduct & Commitment to Maintain Package

I agree to abide by pyOpenSci's Code of Conduct during the review process and in maintaining my package after should it be accepted.
I have read and will commit to package maintenance after the review as per the pyOpenSci Policies Guidelines.

Description

Include a brief paragraph describing what your package does:

FluidImage is a free and open-source Python framework to process images of fluids (in particular with PIV), and analyse the resulting fields.

Fluidimage has now grown into a clean software reimplementing in modern Python algorithms and ideas taken from UVmat, OpenPIV, PIVlab and PIVmat with a focus on performance, usability and maintanability. However, Fluidimage is not restricted to Particle Image Velocimetry computations (PIV, i.e. displacements of pattern obtained by correlations of cropped images) and can be used to (i) display and pre-process images, (ii) compute displacement or velocity fields with PIV, Background-Oriented Schlieren (BOS) and optical flow, and (iii) analyze and display vector and scalar fields.

Scope

Please indicate which category or categories.
Check out our package scope page to learn more about our
scope. (If you are unsure of which category you fit, we suggest you make a pre-submission inquiry):
- Data retrieval
- Data extraction
- Data processing/munging
- Data deposition
- Data validation and testing
- Data visualization[^1]
- Workflow automation
- Citation management and bibliometrics
- Scientific software wrappers
- Database interoperability
For all submissions, explain how and why the package falls under the categories you indicated above. In your explanation, please address the following points (briefly, 1-2 sentences for each):
- Who is the target audience and what are scientific applications of this package?
  
  The target audience is students, scientists, researchers and engineers working with images of fluids. Fluidimage can be used to process images, compute vectors and scalar fields and analyze them.
- Are there other Python packages that accomplish the same thing? If so, how does yours differ?
  
  OpenPIV can be used to compute vectors with various PIV algorithms. Fluidimage is not restricted to PIV. It has a very different API than OpenPIV. Fluidimage has more focus on performance, with in particular compiled extensions produced with Transonic-Pythran and an asynchronous/parallel framework to describe "topologies" of computations and compute then in parallel.

Technical checks

For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:

does not violate the Terms of Service of any service it interacts with.
uses an OSI approved license.
contains a README with instructions for installing the development version.
includes documentation with examples for all functions.
contains a tutorial with examples of its essential functions and uses.
has a test suite.
has continuous integration setup, such as GitHub Actions CircleCI, and/or others.

Publication Options

Do you wish to automatically submit to the Journal of Open Source Software? If so:

I tend to think that Fluidimage is not yet ready for a paper in JOSS. We need more time to get a stronger community and to implement few more features before submitting to JOSS.

JOSS Checks

The package has an obvious research application according to JOSS's definition in their submission requirements. Be aware that completing the pyOpenSci review process does not guarantee acceptance to JOSS. Be sure to read their submission requirements (linked above) if you are interested in submitting to JOSS.
The package is not a "minor utility" as defined by JOSS's submission requirements: "Minor ‘utility’ packages, including ‘thin’ API clients, are not acceptable." pyOpenSci welcomes these packages under "Data Retrieval", but JOSS has slightly different criteria.
The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
The package is deposited in a long-term repository with the DOI:

Note: JOSS accepts our review as theirs. You will NOT need to go through another full review. JOSS will only review your paper.md file. Be sure to link to this pyOpenSci issue when a JOSS issue is opened for your package. Also be sure to tell the JOSS editor that this is a pyOpenSci reviewed package once you reach this step.

Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?

This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.

Yes I am OK with reviewers submitting requested changes as issues to my repo. Reviewers will then link to the issues in their submitted review.

Note that Fluidimage development is done on https://foss.heptapod.net/fluiddyn/fluidimage and that issues have to be filled on this website. Pull requests are of course very welcome. Note that with Heptapod/Mercurial, one should not fork the repo to submit a PR (see https://fluiddyn.readthedocs.io/en/latest/mercurial_heptapod.html).

Confirm each of the following by checking the box.

I have read the author guide.
I expect to maintain this package for at least 2 years and can help find a replacement for the maintainer (team) if needed.

Please fill out our survey

Last but not least please fill out our pre-review survey. This helps us track
submission and improve our peer review process. We will also ask our reviewers
and editors to fill this out.

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

Editor and Review Templates

The editor template can be found here.

The review template can be found here.

The text was updated successfully, but these errors were encountered:

Batalex · 2024-06-22T08:10:54Z

Editor in Chief checks

Hi there! Thank you for submitting your package for pyOpenSci review. Below are the basic checks that your package needs to pass to begin our review. If some of these are missing, we will ask you to work on them before the review process begins.

Please check our Python packaging guide for more information on the elements
below.

Initial onboarding survey was filled out
We appreciate each maintainer of the package filling out this survey individually. 🙌
Thank you authors in advance for setting aside five to ten minutes to do this. It truly helps our organization. 🙌

Editor comments

Hello @paugier, and welcome to pyOpenSci! Sorry it took me so long to get back to you, I have been sitting on these checks for a while now.
fluidimage is truly an interesting project, even from a code structure perspective. I never used mercurial, and I think it's a first for pyOpenSci in general. Same for the licence, never encountered it before!
I'll get started on finding an editor. Thank you for your patience.

cmarmo · 2024-08-13T21:19:45Z

Hello @paugier I'm Chiara, following up as editor in chief during those summer months.
I just want to let you know that you haven't been forgotten ... hope to find an editor for Fluidimage soon 🤞.

cmarmo · 2024-10-01T17:16:43Z

Hello @paugier we are very sorry for making you wait so long!
If you are still interested in our review process (I hope so! 😇), I'm starting looking for reviewers for your submission.

Thanks a lot for your patience! 🙏

cmarmo · 2024-10-20T15:35:58Z

Hello @paugier , I am glad to announce that we have two reviewers for Fluidimage. 🎉

@leahmendelson and @stefanv kindly accepted to take care of this submission: thank you so much to you both! 🙏

Leah, Stefan, before beginning your review, please fill out our pre-review survey. This helps us improve all aspects of our review and better understand our community. No personal data will be shared from this survey - it will only be used in an aggregated format by our Executive Director to improve our processes and programs.

reviewer 1 survey completed.
reviewer 2 survey completed.

The following resources will help you complete your review:

Here is the reviewers guide. This guide contains all of the steps and information needed to complete your review.
Here is the review template that you will need to fill out and submit here as a comment, once your review is complete.

Please get in touch with any questions or concerns!
This review went already a bit out of schedule, I hope a three weeks deadline is ok for you both.
Otherwise please let me know, so we can keep track.

This review is then due on November 10th.

Thank you so much again to you both and to Pierre for his patience!

Happy review to all of you!

cmarmo · 2024-10-20T16:29:47Z

@leahmendelson, I almost forgot.... Do you mind confirming the github handle of your student, so I can add them to the review acknowledgment? Thank you!

leahmendelson · 2024-10-22T00:27:17Z

@emmylia321 is the student who will be co-reviewing with me

cmarmo · 2024-10-22T07:49:17Z

Thank you Leah! And welcome @emmylia321! 👋

paugier · 2024-11-07T12:43:31Z

@emmylia321, @leahmendelson and @stefanv, just to tell you that I've just released a new version of fluidimage (0.5.4) with a small fix of a bug with a new version of Matplotlib. There are also wheels for Python 3.13, but in practice they should not be used before scikit-image 0.25 is really released (only 0.25.0rc1 is available right now).

cmarmo · 2024-11-10T16:07:02Z

Hello dear reviewers, just checking if you had time to move forward with review.
Please let us know! If you need more time I totally understand, I just need to have an insight on the schedule.
@emmylia321, @leahmendelson and @stefanv , thank you for your collaboration!

stefanv · 2024-11-20T05:44:04Z

Hi @paugier! Thanks for your submission. Today, I worked on installing the package from source, and here's an initial set of notes I took while doing so:

The README has a broken link to https://en.wikipedia.org/wiki/Particle_image_velocimetry_(PIV)
Same for docs frontpage.

CeCILL license is, of course fine and choice of the developers, but worth noting that it is more restrictive than the BSD license used by most of the scientific Python ecosystem.

Package minimal dependency for scikit-image is very old; consider updating? See, e.g., SPEC0

Like with the license, Mercurial is fully the developers' choice, but may exclude significant part of ecosystem developers (I used to use Mercurial, but have no idea how to anymore 🤷‍♂️).

Consider shipping example images in separate repository—they really blow up the repo size. See, e.g. pooch.

Consider adding an INSTALL.md or similar with build-from-source instructions; can simply be a link to: https://fluidimage.readthedocs.io/en/latest/build-from-source.html.
Many developers will clone repo and look there for how to proceed.

The installation instructions suggest that pdm install --no-self would create a new virtual env, but instead:

$ pdm install --no-self

INFO: Inside an active virtualenv /home/stefan/envs/py312, reusing it.
Set env var PDM_IGNORE_ACTIVE_VENV to ignore it.

When compiling, lots of Pythran warnings, may be worth reporting to Serge:

  src/fluidimage/calcul/interpolate/__pythran__/thin_plate_spline.cpp:288:13: warning: possibly dangling reference to a temporary [-Wdangling-reference]

skimage now has thin plate splines, btw: https://scikit-image.org/docs/stable/auto_examples/transform/plot_tps_deformation.html
Not sure if that implementation is useful or good enough for this application, but we'd love feedback :)

make tests resulted in a traceback with:

ModuleNotFoundError: No module named 'coverage.results'

Upgrading from coverage 7.6.4 to 7.6.7 worked.
Next, psutil was not found. Huh, something wrong with my psutil install? Reinstalling it fixed the problem.
Next ERROR src/fluidimage - AttributeError: module 'cv2' has no attribute 'imread'.
Reinstalling opencv-python did the trick. OK, at this point I'm thinking pdm may have done something to mess up my environment.
Also reinstalled pillow, scipy, kiwisolver, pyyaml to fix import errors.

Yay, test suite passed! Some warnings to resolve:

src/fluidimage/gui/test_monitor.py: 4 warnings
src/fluidimage/test_run_from_xml.py: 4 warnings
src/fluidimage/topologies/test_bos.py: 2 warnings
src/fluidimage/topologies/test_example.py: 11 warnings
src/fluidimage/topologies/test_image2image.py: 10 warnings
src/fluidimage/topologies/test_mean.py: 6 warnings
src/fluidimage/topologies/test_optical_flow.py: 8 warnings
src/fluidimage/topologies/test_piv.py: 12 warnings
src/fluidimage/topologies/test_preproc.py: 8 warnings
  /usr/lib64/python3.12/multiprocessing/popen_fork.py:66: DeprecationWarning: This process (pid=891926) is multi-threaded, use of fork() may lead to deadlocks in the child.
    self.pid = os.fork()

Will pick up from here soon!

paugier · 2024-11-20T09:45:51Z

@stefanv thanks for looking at this. I'm going to answer in more details and fix things, but it is clear that PDM did not do what it should do in particular because you run it from a virtual env used for something else.

I guess things are going to work much better if you don't do that. PDM should create a dedicated environment in /.../fluidimage/.venv

paugier · 2024-11-20T12:10:39Z

Thanks @stefanv for your notes. I worked on a merge request to improve the situation (https://foss.heptapod.net/fluiddyn/fluidimage/-/merge_requests/116) and opened two issues related to two of your remarks.

The README has a broken link to https://en.wikipedia.org/wiki/Particle_image_velocimetry_(PIV) Same for docs frontpage.

Fixed in https://foss.heptapod.net/fluiddyn/fluidimage/-/merge_requests/116/diffs?commit_id=eaa510beea8938743010f6ef2add5fd2a9e230be

CeCILL license is, of course fine and choice of the developers, but worth noting that it is more restrictive than the BSD license used by most of the scientific Python ecosystem.

CeCILL is equivalent (or better in terms of European laws) than GPL. I wonder how it would be possible than users of a software like fluidimage could be restricted in practice by GPL. However, I asked to the developers if they agree to change to BSD.

Package minimal dependency for scikit-image is very old; consider updating? See, e.g., SPEC0

I updated few minimal dependencies in https://foss.heptapod.net/fluiddyn/fluidimage/-/merge_requests/116/diffs?commit_id=17347a3dca48900011283f2a868b177490d83e52

Like with the license, Mercurial is fully the developers' choice, but may exclude significant part of ecosystem developers (I used to use Mercurial, but have no idea how to anymore 🤷‍♂️).

I have to admit that I doubt that Mercurial could be a barrier for anyone really wanting to contribute to Fluidimage.

for beginners, Mercurial is still simpler than Git
for people used to Git, we have documentation (accessible in particular from our file CONTRIBUTING.md) and our workflow is really simple.

I tend to think that people knowing Git will be able after 2 minutes of reading to run

hg clone ...
cd fluidimage
hg topic my-great-topic-name
hg status
hg commit -m "..."
hg push

More generally, I really think that too much uniformity in tech is bad. For version control, the Git quasi monopoly for open-source is bad. For some aspects, Git is nice, but for some others, it is really bad and old. For example, it is really unfortunate that people cannot safely share history edition and need to push with --force or --force-with-lease. Also the need to use a lot git commit -a is really unfortunate.

Therefore, I consider that it is part of my research tasks to work on one of the alternatives for version control. Mercurial, still used internally at Meta (actually a fork), Google (only the client) and Mozilla, is interesting.

Consider shipping example images in separate repository—they really blow up the repo size. See, e.g. pooch.

This is indeed interesting. A newly cloned repo (cloned in ~ 5 s so it is still fine) has a size of a bit less than 100MB, which is too much. I opened an issue on this https://foss.heptapod.net/fluiddyn/fluidimage/-/issues/42

Consider adding an INSTALL.md or similar with build-from-source instructions; can simply be a link to: https://fluidimage.readthedocs.io/en/latest/build-from-source.html. Many developers will clone repo and look there for how to proceed.

Done in https://foss.heptapod.net/fluiddyn/fluidimage/-/merge_requests/116/diffs?commit_id=677bd1af0f23fa800eac9466a803a629cdf7cb38

The installation instructions suggest that pdm install --no-self would create a new virtual env, but instead:
$ pdm install --no-self

INFO: Inside an active virtualenv /home/stefan/envs/py312, reusing it.
Set env var PDM_IGNORE_ACTIVE_VENV to ignore it.

This is really unfortunate that you experienced such problems! Especially because we use good modern tools and everything works well for us with PDM (and also Pixi).

I updated our documentation to avoid your install issue in https://foss.heptapod.net/fluiddyn/fluidimage/-/merge_requests/116/diffs?commit_id=08b63f2c392d4c5a1e2999f527b2531929cbc568

When compiling, lots of Pythran warnings, may be worth reporting to Serge:

With a virtual env correctly setup with PDM, we also have few warnings related to Pythran, but not the same as you! Anyway, I'm going to report this in https://github.com/serge-sans-paille/pythran/

skimage now has thin plate splines, btw: https://scikit-image.org/docs/stable/auto_examples/transform/plot_tps_deformation.html Not sure if that implementation is useful or good enough for this application, but we'd love feedback :)

I'm going to look at that.

make tests resulted in a traceback with:

Yay, test suite passed! Some warnings to resolve:

All these issues are related to your broken environment. With a correct environment created with PDM, Fluidimage tests pass without warning.

stefanv · 2024-11-20T18:30:18Z

All these issues are related to your broken environment. With a correct environment created with PDM, Fluidimage tests pass without warning.

Great, thanks @paugier! I will rebuild a dedicated virtual environment and re-test. The new documentation says:

You should not run the following commands from a virtual environment not related to Fluidimage.

I think a lot of people start from an existing environment, and after decades of using virtual envs of never tried to get out of one! There is no deactivate command, so not sure how to do that. Looks like pdm --venv .venv or similar may do the trick, though.

Re: the hg and license comments, those were merely observations / data points; totally fine for the project to make opinionated choices, of course. I understand the motivation for libre licenses; one practical issue in our ecosystem is that code cannot flow from GPL packages into BSD libraries. E.g., say you had a good thin-plate splines implementation that we wanted to adopt into scikit-image. But, I'm not pushing you to change the license.

I want to make very clear that my goal with this review is to help the project as much as I can, and to provide some ecosystem context where useful. None of the comments are meant as criticism!

cmarmo · 2024-12-07T22:52:00Z

Hello @leahmendelson @emmylia321 @stefanv , I wonder if we can have the reviews in before Christmas?
Please let me know if you need any help / info.

Thank you so much for volunteering your time for pyOpenSci.

cmarmo · 2025-01-11T13:48:35Z

Hello everybody and happy new year to all of you!
I hope you enjoyed some holiday break, we are now back to business and I wondering if we can set a timeline for the reviews.
@leahmendelson @emmylia321 @stefanv do you mind letting me know if you are able to finalize your review?
Otherwise I can also look for someone else... everyone is busy and this is a volunteer job, so I totally understand if you have issues in moving forward. Please just let us know so we can take the necessary actions.

Thank you so much for your work!

emmylia321 · 2025-01-14T19:31:02Z

Hello! Sorry, for the late response. I believe I can finish the review before January 19th. If that works well for you, I will send it in then. Best, Emmelia Hoorn

…

On Sat, Jan 11, 2025 at 5:48 AM Chiara Marmo ***@***.***> wrote: Hello everybody and happy new year to all of you! I hope you enjoyed some holiday break, we are now back to business and I wondering if we can set a timeline for the reviews. @leahmendelson <https://github.com/leahmendelson> @emmylia321 <https://github.com/emmylia321> @stefanv <https://github.com/stefanv> do you mind letting me know if you are able to finalize your review? Otherwise I can also look for someone else... everyone is busy and this is a volunteer job, so I totally understand if you have issues in moving forward. Please just let us know so we can take the necessary actions. Thank you so much for your work! — Reply to this email directly, view it on GitHub <#194 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BICG5LTTW64M5XPUPPT6KQL2KEOMVAVCNFSM6AAAAABIQ34K2WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBVGI3TKMJVHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

paugier added 0/pre-review-checks New Submission! labels May 30, 2024

github-project-automation bot added this to peer-review-status May 30, 2024

lwasser moved this to under-review in peer-review-status May 30, 2024

Batalex self-assigned this Jun 22, 2024

Batalex added 0/seeking-editor and removed 0/pre-review-checks New Submission! labels Jun 22, 2024

Batalex moved this from under-review to seeking-editor in peer-review-status Jun 22, 2024

cmarmo assigned cmarmo and unassigned Batalex Oct 1, 2024

cmarmo added 1/editor-assigned and removed 0/seeking-editor labels Oct 1, 2024

lwasser moved this from seeking-editor to under-review in peer-review-status Oct 1, 2024

cmarmo added the 2/seeking-reviewers label Oct 7, 2024

cmarmo added 3/reviewers-assigned and removed 2/seeking-reviewers 1/editor-assigned labels Oct 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fluidimage submission #194

Fluidimage submission #194

paugier commented May 30, 2024 •

edited by cmarmo

Loading

Batalex commented Jun 22, 2024 •

edited

Loading

cmarmo commented Aug 13, 2024

cmarmo commented Oct 1, 2024

cmarmo commented Oct 20, 2024 •

edited

Loading

cmarmo commented Oct 20, 2024 •

edited

Loading

leahmendelson commented Oct 22, 2024

cmarmo commented Oct 22, 2024

paugier commented Nov 7, 2024

cmarmo commented Nov 10, 2024

stefanv commented Nov 20, 2024

paugier commented Nov 20, 2024

paugier commented Nov 20, 2024 •

edited

Loading

stefanv commented Nov 20, 2024

cmarmo commented Dec 7, 2024

cmarmo commented Jan 11, 2025

emmylia321 commented Jan 14, 2025 via email

Fluidimage submission #194

Fluidimage submission #194

Comments

paugier commented May 30, 2024 • edited by cmarmo Loading

Code of Conduct & Commitment to Maintain Package

Description

Scope

Technical checks

Publication Options

Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?

Please fill out our survey

Editor and Review Templates

Batalex commented Jun 22, 2024 • edited Loading

Editor in Chief checks

Editor comments

cmarmo commented Aug 13, 2024

cmarmo commented Oct 1, 2024

cmarmo commented Oct 20, 2024 • edited Loading

cmarmo commented Oct 20, 2024 • edited Loading

leahmendelson commented Oct 22, 2024

cmarmo commented Oct 22, 2024

paugier commented Nov 7, 2024

cmarmo commented Nov 10, 2024

stefanv commented Nov 20, 2024

paugier commented Nov 20, 2024

paugier commented Nov 20, 2024 • edited Loading

stefanv commented Nov 20, 2024

cmarmo commented Dec 7, 2024

cmarmo commented Jan 11, 2025

emmylia321 commented Jan 14, 2025 via email

paugier commented May 30, 2024 •

edited by cmarmo

Loading

Batalex commented Jun 22, 2024 •

edited

Loading

cmarmo commented Oct 20, 2024 •

edited

Loading

cmarmo commented Oct 20, 2024 •

edited

Loading

paugier commented Nov 20, 2024 •

edited

Loading