[REVIEW]: pyFBS: A Python package for Frequency Based Substructuring

whedon commented 3 years ago

Submitting author: @tb93 (Tomaž Bregar) Repository: https://gitlab.com/pyFBS/pyFBS Version: 0.1.1 Editor: @kyleniemeyer Reviewer: @ngoiz, @LolloCappo, @Gunnstein Archive: 10.5281/zenodo.5876734

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/0c3871be156ceda663f7cee8420a1c84"><img src="https://joss.theoj.org/papers/0c3871be156ceda663f7cee8420a1c84/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/0c3871be156ceda663f7cee8420a1c84/status.svg)](https://joss.theoj.org/papers/0c3871be156ceda663f7cee8420a1c84)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@ngoiz & @LolloCappo & @Gunnstein, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

Make sure you're logged in to your GitHub account
Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @kyleniemeyer know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Review checklist for @ngoiz

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@tb93) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[ ] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[ ] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of Need' that clearly states what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[ ] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @LolloCappo

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@tb93) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of Need' that clearly states what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @Gunnstein

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@tb93) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[ ] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[ ] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of Need' that clearly states what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[ ] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @ngoiz, @LolloCappo, @Gunnstein it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf

whedon commented 3 years ago

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.11 s (767.7 files/s, 176938.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          16            957           1426           2095
Jupyter Notebook                21              0          10037           1772
reStructuredText                33            917           1000            637
TeX                              2             28              0            231
HTML                             6             15              0            193
Markdown                         1             26              0             88
YAML                             2             15             16             43
DOS Batch                        1              8              1             27
CSS                              1              4              4             13
INI                              1              5              0             13
make                             1              4              6             10
-------------------------------------------------------------------------------
SUM:                            85           1979          12490           5122
-------------------------------------------------------------------------------

Statistical information for the repository '05048cb517afec299ab5068e' was
gathered on 2021/06/23.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Domen Ocepek                     4           517              8            0.71
Francesco Trainotti              2            48            260            0.42
Locke-D-A                        3          1340            706            2.78
Miha                             3             1             63            0.09
Tomaž BRegar                     3            86              8            0.13
Tomaž Bregar                   112         33908          32986           90.79
d.ocepek                         1            23              6            0.04
mihakodric                      17           807            457            1.72
mihapogacar                      3           255            149            0.55
tb93                             3          2018             31            2.78

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Domen Ocepek                511           98.8          2.7               15.07
Francesco Trainotti          24           50.0          2.6                0.00
Locke-D-A                     2            0.1         12.0                0.00
Miha                          1          100.0          0.2                0.00
Miha Kodrič                 366          100.0          4.7                3.01
Tomaž Bregar               3365            9.9         12.4               14.56
mihapogacar                 209           82.0          0.2               11.96

whedon commented 3 years ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- None

MISSING DOIs

- 10.1016/j.ymssp.2017.12.003 may be a valid DOI for title: System equivalent model mixing
- 10.21105/joss.01450 may be a valid DOI for title: PyVista: 3D plotting and mesh analysis through a streamlined interface for the Visualization Toolkit (VTK)
- 10.2514/1.33274 may be a valid DOI for title: General Framework for Dynamic Substructuring: History,  Review and Classification of Techniques
- 10.1016/j.ymssp.2015.08.004 may be a valid DOI for title: General framework for transfer path analysis: History,  theory and classification of techniques
- 10.1016/j.ymssp.2019.106440 may be a valid DOI for title: Including directly measured rotations in the virtual point transformation
- 10.4271/720093 may be a valid DOI for title: A combined experimental and analytical procedure for improving automotive system dynamics
- 10.2514/6.1984-941 may be a valid DOI for title: Combined Experimental/Analytical Modelling using component modes synthesis

INVALID DOIs

- None

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

kyleniemeyer commented 3 years ago

:wave: @ngoiz @LolloCappo @Gunnstein @tb93 The actual review will take place in this issue.

Each reviewer has a separate checklist at the top, which they can work through; our reviewer guidelines may also be helpful. General comments and feedback can be given as comments in this issue, and if appropriate reviewers can open issues on the software repository.

Thanks again!

kyleniemeyer commented 3 years ago

@tb93 while the reviewers begin their work, it looks like your article is missing DOIs for some/all references. Please add those when you get a chance.

tomazbregar commented 3 years ago

Thank you @kyleniemeyer for your remark regarding the missing DOIs, we updated the references.

tomazbregar commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

whedon commented 3 years ago

:wave: @ngoiz, please update us on how your review is going (this is an automated reminder).

whedon commented 3 years ago

:wave: @Gunnstein, please update us on how your review is going (this is an automated reminder).

ngoiz commented 3 years ago

Hello,

I have now gone through the review process for pyFBS.

First, let me congratulate you for the tool, it is simple to use, provides great functionality and is technically very complete. In particular, let me praise the quality of the example jupyter notebooks and the effort that has gone into creating those. For a new user to the code, they are immensely helpful.

As part of my review, I only have minor comments related to the following items:

Installation instructions: (issue in repository) it took me a while to install pyFBS, most likely because of outdated dependencies on my system. A minimum set of requirements to run pyFBS and supported operating systems would be extremely helpful in the documentation section of the install.
Community guidelines: you use a separate repository for support issues. A small entry in the documentation on how to contribute to the code to comply with your ways of working would be excellent.
References: extremely minor but would be good to fix it for the final paper, there is a missing autoref in line 54 of the paper autoref{fig:3D} which is not rendering properly.

Other than that, I enjoyed going through your tool and again, well done on the examples.

Thanks, Norberto

danielskatz commented 3 years ago

👋 @kyleniemeyer - How is this review going?

kyleniemeyer commented 3 years ago

@ngoiz thank you for your comments!

@tb93 As you can see above, @ngoiz left a few comments/suggestions for you. Can you please address these?

@Gunnstein have you been able to review this submission?

Gunnstein commented 3 years ago

Dear Kyle

@ngoiz thank you for your comments!

@tb93 As you can see above, @ngoiz left a few comments/suggestions for you. Can you please address these?

@Gunnstein have you been able to review this submission?

Dear @kyleniemeyer,

I am working on the review now. I am not able to edit the review list, the link for review invitation is not valid, can you send a new one?

Best regards, Gunnstein

kyleniemeyer commented 3 years ago

@whedon re-invite @Gunnstein as reviewer

whedon commented 3 years ago

OK, the reviewer has been re-invited.

@gunnstein please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

kyleniemeyer commented 3 years ago

@Gunnstein thanks for the update. If you click that link your invitation should be reactivated.

Gunnstein commented 3 years ago

Dear all,

The package is a unique open source project in dynamic substructuring and is an important contribution to engineers and researchers in dynamics and the open source community. The overall design of the package is intuitive and it follows well established coding practice which makes it easy to understand and contribute to the project. The package also has concise documentation with useful examples that help new users evaluate and to start applying the software.

Comments associated with review:

Automated tests and Community guidelines:

No description of the testing regime is provided. A test suite based on pytest and tox is provided. Experienced users will know how to perform tests, but a description of the manual steps required to perform testing will help contributors and maintainers (avoid non-passing pull requests).
No guidelines for contributing to the package is provided. What coding style, documentation format, cross platform compatibility, etc.? How are contributions made and bugs/issues handled? It is also natural to provide the description and requirements of tests/testing here.

References:

Line 54 of paper has a misshandled reference: "autoref{fig:3D}"

A comment on security:

Several examples (and provided example data?), tests and the MCK module use pickle for data storage/caching. Pickling represents a security issue where arbitrary code can be executed at load time. Practically it makes it challenging to maintain the package as any contribution on these elements are potential security risks. It will also stop certain users adopting the package entirely.

Since many of the users of this package may not be familiar with the security issue, a notification is appropriate in the docstring of methods/functions and at the top of example notebooks. It also seems like pickling can be avoided entirely in case of the MCK module since the data objects are essentially arrays and vectors which can readily be stored without pickling.

Best regards, Gunnstein

kyleniemeyer commented 2 years ago

Thank you for this review @Gunnstein!

Hello @tb93, we now have a complete set of reviews, and a few items for you to address before accepting your submission. Are you able to address / work on these things?

tomazbregar commented 2 years ago

Hey @kyleniemeyer, we are working on it. We will try to finalise all the outlined points as fast as possible.

tomazbregar commented 2 years ago

Dear all,

sorry for the late response from our side. We have overhauled our documentation site, where we also added new examples and use-cases of the whole package.

Paper The mishandled reference was corrected.

Contributing We have added a detailed Contribute section within documentation.

Testing Instructions about testing were added in the Contribute section.

Installation With the current version, Python 3.8 should be used and the PyPi installation works without any problem. Nevertheless, we should be able to support Python 3.9 in the upcoming release.

Pickle issue A warning was added to the documentation - . However, in long term we already have a plan to implement hdf file format instead of pickle for file saving.

Best regards, Tomaž

tomazbregar commented 2 years ago

Dear @kyleniemeyer, what is the next step in the revision of the joss submission? We updated the documentation of the package.

Sincerely, Tomaž

kyleniemeyer commented 2 years ago

@tomazbregar sorry about that, I missed your update due to the end of our fall term and the holidays. I will review your updates.

kyleniemeyer commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

kyleniemeyer commented 2 years ago

Hi @tomazbregar, I have reviewed your changes and I believe you have addressed the outstanding concerns.

I just submitted a PR for your paper that fixes some reference formatting and a few minor typos, can you please merge this? https://gitlab.com/pyFBS/pyFBS/-/merge_requests/15

Once you have merged this, please archive your repository (using Zenodo or Figshare) and provide the DOI here.

tomazbregar commented 2 years ago

Hey @kyleniemeyer, thank you for finalizing the review. I merged your PR, thank you for this.

I have archived the current version of the repository - https://zenodo.org/record/5863174.

The DOI number:

kyleniemeyer commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

kyleniemeyer commented 2 years ago

@tomazbregar it looks like the authors associated with the Zenodo archive do not match those in the paper. Normally, these two author lists should be the same. Can you check this?

tomazbregar commented 2 years ago

Hey @kyleniemeyer, you are correct. When we started to write the paper, we only had a limited team developing the package (3 PhDs studends), and now the team expanded, but we did not change the paper. How do you commonly handle those things?

kyleniemeyer commented 2 years ago

@tomazbregar totally understandable, especially since this review has taken so long.

If the new team members have contributed to work that is reflected in the work considered here, in the version of the software that was reviewed and will be published, then they should be added to the author list of the paper. Since the Zenodo archive should only contain the latest version of the software that has been reviewed, usually these lists should match, and the paper author list be updated to reflect the newer work.

If your software has newer work that was not reviewed, then I would ask that you instead upload a snapshot of the repository that reflects the reviewed version, and have the Zenodo author list match.

It also looks like the last three authors on the paper are missing from the Zenodo archive—they should be added either way.

Thanks!

tomazbregar commented 2 years ago

Hey @kyleniemeyer, thank you for the explanation. We made quite a lot of changes to the software, since we first started to write the paper. Therefore, we also included the new members in the Joss paper. The updated version is already on master branch and I also uploaded the new archive to zenodo (doi number down bellow). Now the authors match on the repository and on the joss paper.

kyleniemeyer commented 2 years ago

@tomazbregar great, thanks

kyleniemeyer commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

kyleniemeyer commented 2 years ago

@whedon set 10.5281/zenodo.5876734 as archive

whedon commented 2 years ago

OK. 10.5281/zenodo.5876734 is the archive.

kyleniemeyer commented 2 years ago

@whedon accept

whedon commented 2 years ago

To recommend a paper to be accepted use @whedon recommend-accept

kyleniemeyer commented 2 years ago

@whedon recommend-accept

whedon commented 2 years ago

Attempting dry run of processing paper acceptance...

whedon commented 2 years ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.ymssp.2017.12.003 is OK
- 10.21105/joss.01450 is OK
- 10.2514/1.33274 is OK
- 10.1016/j.ymssp.2015.08.004 is OK
- 10.1016/j.ymssp.2019.106440 is OK
- 10.4271/720093 is OK
- 10.2514/6.1984-941 is OK

MISSING DOIs

- None

INVALID DOIs

- None

whedon commented 2 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/2896

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/2896, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true

kyleniemeyer commented 2 years ago

@whedon accept deposit=true

whedon commented 2 years ago

Doing it live! Attempting automated processing of paper acceptance...

whedon commented 2 years ago

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

whedon commented 2 years ago

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/2897
Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.03399
If everything looks good, then close this review issue.
Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

kyleniemeyer commented 2 years ago

Congratulations @tb93 on your article's publication in JOSS, and thank you for your patience for such a long journey!

Many thanks to @ngoiz, @LolloCappo, and @Gunnstein for reviewing this submission.

openjournals / joss-reviews