pyOpenSci / software-submission

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

CovPlan #170

Open sanjeevrs2000 opened 4 months ago

sanjeevrs2000 commented 4 months ago

Submitting Author: Name (@sanjeevrs2000) All current maintainers: (@sanjeevrs2000) Package Name: CovPlan One-Line Description of Package: A Python package that generates guidance trajectories for field coverage using a single robot. Repository Link: https://github.com/sanjeevrs2000/covplan Version submitted: 0.1.0 Editor: TBD
Reviewer 1: TBD
Reviewer 2: TBD
Archive: TBD JOSS DOI: TBD Version accepted: TBD Date accepted (month/day/year): TBD


Code of Conduct & Commitment to Maintain Package

Description

Scope

Domain Specific

Community Partnerships

If your package is associated with an existing community please check below:

[^1]: Please fill out a pre-submission inquiry before submitting a data visualization package.

It can be classified under applications in AI and robotics. Unsure which of the above categories it comes under but were encouraged to make a submission in the presubmission enquiry. Researchers in coverage path planning might find it useful for developing new algorithms or comparing it against their own methods. It has potential applications in field robotics where coverage of an area is required for monitoring, information gathering tasks.

Technical checks

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

Publication Options

JOSS Checks - [ ] The package has an **obvious research application** according to JOSS's definition in their [submission requirements][JossSubmissionRequirements]. 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][JossSubmissionRequirements]: "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][JossPaperRequirements] 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.

Confirm each of the following by checking the box.

Please fill out our survey

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.

isabelizimm commented 4 months ago

Welcome to pyOpenSci--we are so glad you are here! Thank you for this submission, just letting you know we have seen this issue and will get back to you with pre-review checks shortly. 🌻

Gonzalo-Mier commented 3 months ago

Hi @sanjeevrs2000, the CovPlan seems a cool repo.

I've found the mention to the Fields2Cover project and I would add some info to your claim:

Fields2Cover is a similar implementation although it is in C++.

Fields2Cover provides is implemented in C++, and provides a Python interface using swig

Our work also considers more complex areas for coverage such as ones with forbidden regions and solves it by using a divide and conquer approach.

This has been included in fields2cover v2 (released this week).

It is also not limited to agriculture related applications and can be adapted for other uses by modifying the parameters

Same as Fields2Cover. The focus is agriculture, but it is not limited.

Other Python package that implement coverage path planners: https://github.com/RuslanAgishev/motion_planning

Btw, congrats for the @pyOpenSci project to their authors!

sanjeevrs2000 commented 3 months ago

Hi @Gonzalo-Mier, thanks a lot for the info. We were not aware of these recent developments with Fields2Cover. Keep up the great work with your repo!

Batalex commented 3 months ago

Hello @sanjeevrs2000, Thank you for your patience. You may not know, but we now implement a 3-month rotation for the editor-in-chief role. I'll get started with the EiC checks right now.

Batalex commented 3 months ago

Hi @Gonzalo-Mier, welcome to pyOpenSci!

Thanks for the info. As @NickleDave reminded me, Fields2Cover was mentioned in the presubmission inquiry a few months ago. As we already decided that CovPlan was in scope, we most likely will proceed with the review.

Now for the shameless part 😁
Though we do try to avoid duplication of efforts (hence our question on similar packages), we do not position ourselves as an authority on which package should be the reference. If anything, I would love for this GH issue to be the starting point of a collaborative effort that would benefit everyone. If you are interested and available, please let me know!

Batalex commented 3 months ago

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.



Editor comments

As of now, there are too many missing things to get started with the review. Some are really trivial (like adding a CONTRIBUTING.md or restructuring the readme using the suggestion below), others require additional work.

I encourage you to go through our Python Package Guide to get familiar with:

README.md suggested structure

Expand ````md # CovPlan A Python package for coverage path planning. [![MIT License](https://img.shields.io/badge/License-MIT-green.svg)](https://choosealicense.com/licenses/mit/) A brief description of what this project does and who it's for. Link to paper + full citation > Hameed IA, Bochtis D, Sørensen CA. An Optimized Field Coverage Planning Approach for Navigation of Agricultural Robots in Fields Involving Obstacle Areas. International Journal of Advanced Robotic Systems. 2013;10(5). doi:10.5772/56248 ## Installation Install `covplan` with pip ```shell pip install covplan ``` ## Getting started The 30 seconds tutorial. ```py import antigravity ``` ## Documentation [Documentation](https://linktodocumentation) ## Contributing Contributions are always welcome! See `contributing.md` for ways to get started. Please adhere to this project's `code of conduct`. ````
Gonzalo-Mier commented 3 months ago

Hi @Batalex,

My comment was more related with correct info related to the project than giving reasons to dismiss the CovPlan. I would love to see it progress. Answering your comments:

Though we do try to avoid duplication of efforts (hence our question on similar packages), we do not position ourselves as an authority on which package should be the reference.

Nor my intention either. More, better!

As we already decided that CovPlan was in scope, we most likely will proceed with the review.

Of course, it is certainly a nice addition to the open-source ecosystem and @sanjeevrs2000 is doing an awesome job.

If anything, I would love for this GH issue to be the starting point of a collaborative effort that would benefit everyone. If you are interested and available, please let me know!

I don't have too much free time, but I can help if needed. Please, contact me for any collaboration :)

sanjeevrs2000 commented 3 months ago

Hi @Gonzalo-Mier, thanks again for your support. I am not a software developer myself so I have limited time as well. Will be happy to collaborate if required.

I don't have too much free time, but I can help if needed. Please, contact me for any collaboration :)

@Batalex, if it is decided that it is under scope for a review, we shall try to meet these requirements as soon as possible.

Some are really trivial (like adding a CONTRIBUTING.md or restructuring the readme using the suggestion below), others require additional work

sanjeevrs2000 commented 2 months ago

Hi @Batalex, I've added tests and updated the documentation as per the recommendations. Hopefully, this is enough to get started with the review now. Let me know if you find any other areas for improvement.

Batalex commented 2 months ago

Hi @sanjeevrs2000, Thank you for your work on the package. There are still some things missing before I can give the go for the review.

Documentation

Since you are using mkdocs, I advise you to take a look at https://mkdocstrings.github.io/. It is a plugin that automatically picks up the docstrings to embed them in your documentation. The reason why I ask you to do that is because it will help you keep your code base and your documentation in sync.
Right now, the API reference looks like it's been written manually, and it is a problem. See, the code base is already out of sync with the documentation, and this issue will only continue to grow with the code base and the number of contributors to the project.

I also noticed a lack of consistency in the docstrings formats: some use Numpy/Google convention, others use the classic Python one I linked in my previous post. The function linked above uses neither.

You might want to explicitly list the development dependencies one needs to work on the project. It happens that I am quite familiar with mkdocs so I already know how to build the documentations, but that most likely not the case for everyone. To do so, you may:

Tests and automation

I am glad to see that you added tests, but we need to go one step further. Because tests are only as useful as their consistent runs, we need to integrate them into GitHub. Even before that, same as the previous section, it is a good idea to include a mention of how to run the tests in your contributing.md file. Same as above, you will need to explicitly state your test dependencies if you have any.

Back to GitHub, the aim of the continuous integration (CI), is to make sure that you do not introduce any regression to the code base, using the tests you wrote. By making GitHub run your tests on every pull request (and I advise you to use this workflow from now on even if you are the sole contributor), you can merge new additions to the code base more confidently.

We wrote a guide for that, this is a great starting point.

They are a few more things we could work on, but I don't want to move the goal posts further with every interaction, so I'll leave it there for now.