search

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

automata #152

Closed eliotwrobson closed 3 months ago

eliotwrobson commented 10 months ago

Submitting Author: Eliot Robson (@eliotwrobson) All current maintainers: (@eliotwrobson, @caleb531) Package Name: automata One-Line Description of Package: A Python library for simulating finite automata, pushdown automata, and Turing machines. Repository Link: https://github.com/caleb531/automata Version submitted: v8.2.0 EiC: @isabelizimm Editor: @sneakers-the-rat
Reviewers: @phildong , @irisdyoung, @NimaSarajpoor Reviews Expected By: March 28th, 2024 Archive: DOI Version accepted: 8.4.0 (repo, pypi, archive) Date accepted (month/day/year): 06/29/2024

Code of Conduct & Commitment to Maintain Package

Description

Automata is a Python 3 library implementing structures and algorithms for manipulating finite automata, pushdown automata, and Turing machines. The algorithms have been optimized and are capable of processing large inputs. Visualization logic has also been implemented.

Scope

Domain Specific & Community Partnerships 

- [ ] Geospatial
- [x] Education
- [ ] Pangeo

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.

This package is suitable for both researchers wishing to manipulate automata and for instructors teaching courses on theoretical computer science. Automata (especially finite automata) are important models in computing that appear in a variety of educational and research contexts, and the ability to manipulate them with this package is valuable to this effort.

There are some smaller packages with similar scope (for example here), but automata is the most popular, best maintained, and most feature-rich.

https://github.com/pyOpenSci/software-submission/issues/135

Although I am not the primary repository owner (that is @caleb531), he has given me permission to make this submission and be a long-term point of contact as-needed. Additionally, our documentation page is brand-new, so we anticipate some rough edges, and hope that feedback from this review can be used to improve the usability and add examples. Automata also already appeared in JOSS, so that is why those items have been left blank here even though the writeup is present in the repository.

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.

lwasser commented 10 months ago

hey there @eliotwrobson 👋 Welcome to pyOpenSci!! I just wanted to let you know that we have seen this submission and will be following up in the next week about moving forward with it! Have a wonderful weekend. ✨

lwasser commented 9 months ago

hi again @eliotwrobson @caleb531 👋 my apologies for the delay! we are making a few changes to our editorial process and that took a bit of time to sort out!

Our Editor in Chief for the next quarter, @isabelizimm will be running initial checks on this package in the upcoming week. After that @sneakers-the-rat will lead the review as editor! You will hear from our editorial team sometime in the next week to kick off the review. ✨ Thank you for your patience!

isabelizimm commented 9 months ago

Thank you so much for this submission! I've done some preliminary checks below, which all look great 🎉 your editor will be @sneakers-the-rat, who will take it from here with finding reviewers.

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.

lwasser commented 9 months ago

hi there @eliotwrobson 👋 i wanted to point out two small items that you could address associated with your package's structure!

  1. you have a shell setup.py file here that you don't need! It did used to be the case that the setup.py file was needed for editable installs. HOwever that is no longer the case. You can remove it and all should be fine.
  2. Setuptools also installs wheel at runtime as a requirement. so you can remove wheel from your pyproject.toml build declaration

These are small changes but will modernize / clean up your package just a bit! :)

eliotwrobson commented 8 months ago

@sneakers-the-rat are there any more action items we can handle before reviews start? Just wanted to check in, thanks!

sneakers-the-rat commented 8 months ago

Hello! sorry for the delay, i've been prepping for/at a workshop the last few weeks, but let's get this rolling. I have one reviewer that has previously agreed to review this, and am putting the call out for more.

phildong commented 8 months ago

Hello this is the "one reviewer" popping in and saying Hi. Very excited for this and ready to jump in whenever you're ready!

sneakers-the-rat commented 8 months ago

Hello and thank you for agreeing to review @phildong !!!!!

Still casting about for reviewers, I see @nathanael-fijalkow and @rohaquinlop as having worked on similar projects in the past by searching through github a bit, although they have not agreed to review for pyopensci, will ask again on masto and in reviewer channels :)

sneakers-the-rat commented 7 months ago

Checking in again, I'll make another call for a second reviewer, but @eliotwrobson if you could recommend anyone that might be suitable as a reviewer that would be great!

eliotwrobson commented 7 months ago

@sneakers-the-rat thanks for checking in! I don't have anyone specific in mind (I'm honestly not sure what the qualifications are for review). If you wanted to, you could ask people that did the previous JOSS review? That might make things easier since they already know their way around.

sneakers-the-rat commented 7 months ago

We have located a second reviewer! Thank you to @irisdyoung for volunteering, and thanks @phildong for your patience :)

Now that we have two reviewers, let's get started!

pyOpenSci is a young-ish organization and doesn't have some of the fancier review bot features set up like JOSS yet (we're working on it!), but the reviews work similarly if you've done that before:

First, check out the reviewer guide here - https://www.pyopensci.org/software-peer-review/how-to/reviewer-guide.html (and you can raise issues if anything is unclear here: https://github.com/pyOpenSci/software-peer-review ). There are example reviews linked from there, and you can also see previous reviews i have done for pyopensci (pygraphblas) or joss (reorient)

My role here is to make sure the reviewers have everything needed to complete the review, and make sure the vibes stay close to a supportive, collaborative discussion. So I don't end up typing this 50 times: if you need anything at all, don't hesitate to ask. If you would prefer speaking privately, you can reach me on the fediverse or via email.

As an overview, you are going to do three things

As reviewers, you are taking the role of an interested and critical potential user - it should be possible for you to (non-exhaustively) a) understand what the package is supposed to be able to do, b) know how to do it, c) be able to do it, and d) be confident that functionality won't break. All that should require no special information from the authors - you are reviewing both for technical correctness as well as quality of documentation and technical infrastructure like CI. A decent heuristic is, by the end of the review, the package should be one that you would want to add as a dependency in your work.

The checklist is a minimum bar, but you are welcome to review anything beyond that - as reviewers you can coordinate amongst yourselves if one or the other of you has a preferred focus, eg. someone prefers reviewing for docs and the other wants to review for performance, etc. You can signal if a particular issue is a blocker for you or not, and we can resolve those as they come up - optional suggestions, comments, questions, etc. are also in bounds.

This is an open, collaborative review process - the checklist is about bringing packages in python world up to a minimal standard of maintainability and functionality, but it is not a gate to keep. There is no concept of "rejection" here, instead we are trying to help the authors reach that standard. Be the code reviewer you always wished you had <3.

And with that lengthy explanation... let's get started. the first thing you'll both want to do is copy and paste the review template (also added in the collapsible below) into a new comment that you can then edit as you do your review :) glhf <3

Let's set a tentative date to expect reviews 3 weeks from now - March 28th

Expand/collapse review template ``` ## Package Review *Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide* - [ ] As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor _before_ starting your review). #### Documentation The package includes all the following forms of documentation: - [ ] **A statement of need** clearly stating problems the software is designed to solve and its target audience in README. - [ ] **Installation instructions:** for the development version of the package and any non-standard dependencies in README. - [ ] **Vignette(s)** demonstrating major functionality that runs successfully locally. - [ ] **Function Documentation:** for all user-facing functions. - [ ] **Examples** for all user-facing functions. - [ ] **Community guidelines** including contribution guidelines in the README or CONTRIBUTING. - [ ] **Metadata** including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a `pyproject.toml` file or elsewhere. Readme file requirements The package meets the readme requirements below: - [ ] Package has a README.md file in the root directory. The README should include, from top to bottom: - [ ] The package name - [ ] Badges for: - [ ] Continuous integration and test coverage, - [ ] Docs building (if you have a documentation website), - [ ] A [repostatus.org](https://www.repostatus.org/) badge, - [ ] Python versions supported, - [ ] Current package version (on PyPI / Conda). *NOTE: If the README has many more badges, you might want to consider using a table for badges: [see this example](https://github.com/ropensci/drake). Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)* - [ ] Short description of package goals. - [ ] Package installation instructions - [ ] Any additional setup required to use the package (authentication tokens, etc.) - [ ] Descriptive links to all vignettes. If the package is small, there may only be a need for one vignette which could be placed in the README.md file. - [ ] Brief demonstration of package usage (as it makes sense - links to vignettes could also suffice here if package description is clear) - [ ] Link to your documentation website. - [ ] If applicable, how the package compares to other similar packages and/or how it relates to other packages in the scientific ecosystem. - [ ] Citation information #### Usability Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider whether: - [ ] Package documentation is clear and easy to find and use. - [ ] The need for the package is clear - [ ] All functions have documentation and associated examples for use - [ ] The package is easy to install #### Functionality - [ ] **Installation:** Installation succeeds as documented. - [ ] **Functionality:** Any functional claims of the software been confirmed. - [ ] **Performance:** Any performance claims of the software been confirmed. - [ ] **Automated tests:** - [ ] All tests pass on the reviewer's local machine for the package version submitted by the author. Ideally this should be a tagged version making it easy for reviewers to install. - [ ] Tests cover essential functions of the package and a reasonable range of inputs and conditions. - [ ] **Continuous Integration:** Has continuous integration setup (We suggest using Github actions but any CI platform is acceptable for review) - [ ] **Packaging guidelines**: The package conforms to the pyOpenSci [packaging guidelines](https://www.pyopensci.org/python-package-guide). A few notable highlights to look at: - [ ] Package supports modern versions of Python and not [End of life versions](https://endoflife.date/python). - [ ] Code format is standard throughout package and follows PEP 8 guidelines (CI tests for linting pass) #### For packages also submitting to JOSS - [ ] The package has an **obvious research application** according to JOSS's definition in their [submission requirements](http://joss.theoj.org/about#submission_requirements). *Note:* Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted. The package contains a `paper.md` matching [JOSS's requirements](http://joss.theoj.org/about#paper_structure) with: - [ ] **A short summary** describing the high-level functionality of the software - [ ] **Authors:** A list of authors with their affiliations - [ ] **A statement of need** clearly stating problems the software is designed to solve and its target audience. - [ ] **References:** With DOIs for all those that have one (e.g. papers, datasets, software). #### Final approval (post-review) - [ ] **The author has responded to my review and made changes to my satisfaction. I recommend approving this package.** Estimated hours spent reviewing: --- #### Review Comments ```
sneakers-the-rat commented 7 months ago

Checking in with reviewers, @phildong @irisdyoung - do we think we'll need more time?

phildong commented 7 months ago

@sneakers-the-rat sorry I was traveling last week. I aim to finish the review this week/weekend!

phildong commented 7 months ago

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

Documentation

The package includes all the following forms of documentation:

Readme file requirements The package meets the readme requirements below:

The README should include, from top to bottom:

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider whether:

Functionality

Final approval (post-review)

Estimated hours spent reviewing:

10hr

Review Comments

The automata package allow the users to easily build and manipulate finite automata, pushdown automata, and Turing machines. It's an invaluable tool for teaching and demonstration of this topic. The algorithm is also optimized for large inputs so it can be useful in theoretical research. The code is well-packaged with detailed documentation and CI setup. I was able to easily install and use the package.

Specific comments:

phildong commented 7 months ago

ok finished my first pass of review. @sneakers-the-rat since it's first time I do this please let me know if I'm on the right track and if there's any major stuff I'm missing!

Meanwhile I got some questions:

sneakers-the-rat commented 7 months ago

Looks great so far @phildong -

Am I supposed evaluate the novelty/impact the package?

there has been some ongoing discussion about whether this should matter in pyopensci reviews - at the moment the answer is no, don't worry about novelty/impact. The EiC has already cleared the package as in-scope. You are welcome to comment about this in your review, as well as request links to prior work as relevant: putting work in context is an important part of reviewing, and there is one item in the checklist asking about "how the package compares to other similar packages." It is a generally good practice for packages to maintain a "see also" section in their docs if there are other packages that are closely related, along with a note explaining similarities and differences: "Our package is for x, if you are looking for y then consider z package," and so on. You are also welcome to open an issue asking the authors about this if you aren't sure about the relation to prior work!

I see the "JOSS publication" section was not checked in the original submission, so do I still need to worry about the corresponding section in the review template?

nope :) you can remove those checkboxes from your review.

So I'm wondering what's the criteria for examples and what's the best way to proceed?

This is a tough judgement call to make. re: API documentation - what counts as a package's public API anyway? A decent heuristic is "the set of functions that one would expect to use to accomplish the stated purpose of the package." So even if they aren't marked private by a leading underscore, you might not require docstrings for every function in a utils module if they would only be used in an edge case, or are mostly used internally within the package. Another heuristic is 'do these functions show up in example notebooks' (aside from, well, utility functions that are explicitly for use in notebooks eg. for pretty printing and the like). Ultimately it's up to you as the reviewer whether the package is documented in a way that allows you to evaluate it, and would allow your average bear to use it.

eliotwrobson commented 6 months ago

@sneakers-the-rat any updates on the review from the second reviewer? We already addressed one open issue from the first review, and it would be helpful to have all issues / comments from this review before we make a pass through everything.

eliotwrobson commented 6 months ago

@sneakers-the-rat (cc @irisdyoung) do you think we might be able to close out reviews soon? The end of the academic year is coming up for me, and it would be awesome to be able to close out the remaining items for this review before the start of the summer.

sneakers-the-rat commented 6 months ago

Apologies, have been afk and traveling for the last few weeks. Back at my desk tomorrow and yes lets scoot this along :)

sneakers-the-rat commented 6 months ago

OK - checking in with @irisdyoung offline, @phildong is finished with first pass of review, so for that we'd be waiting on any final issues to raise and otherwise just a final review statement that summarizes a) outcomes of review checklist, b) overall impressions of package, c) strengths, and d) opportunities for growth :). I'll check in with phil separately monday if he's not checking on his gh notifications.

edit: will follow up with another timeline after making contact with reviewers

sneakers-the-rat commented 6 months ago

Checking in again - @irisdyoung any estimate on review time? and @phildong for some summary statement on the package if you have seen enough of it?

eliotwrobson commented 5 months ago

@sneakers-the-rat just a heads up that I'm going to get started on the existing open issues from this review pretty soon, along with expanding the examples in the documentation (based on the unchecked boxes left in the open review).

phildong commented 5 months ago

Sorry for the silence! I have followed up in the individual issues/PRs to provide some clarification (hopefully). Once they're addressed/closed I'm happy to write up a summary and make that final tick :smile:

sneakers-the-rat commented 5 months ago

@irisdyoung last call for review :) no worries if things are too hectic on your end, just want to know to start looking for another reviewer.

irisdyoung commented 5 months ago

Hello and apologies for the radio silence over here. I've done a first look through and can plan to raise any issues before the weekend. Talk soon!

eliotwrobson commented 5 months ago

Docs PR addressing most of the currently open issues from this review: https://github.com/caleb531/automata/pull/217

Basically adds discussion addressing most of the points of confusion that have come up so far.

Regarding more self-contained tutorials, that's coming up soon 👍👍

sneakers-the-rat commented 5 months ago

Great @irisdyoung thanks for letting us know.

eliotwrobson commented 5 months ago

@irisdyoung any updates? I think we've addressed all the issues you opened.

lwasser commented 4 months ago

hi there team 👋🏻 I wanted to check in on this review. your fearless editor @sneakers-the-rat is focused on other more pressing issues now going on in their professional environment. As such i'm just checking in to see if we can support Jonny in moving this review forward while they focus on bigger more important things.

As such, i'm trying to figure out the state of this review! i see one review submitted from @phildong -- thank you!! @irisdyoung i suspect you submitted some issues around the package but i don't see your format review submission using this template (please excuse me if i missed it!! )

it's important to have a tracebable record of the review so we ask that you both submit a formal review as a comment using that template and if you open issues / pr's (amazing!!) please link them back to this issue for traceability!

in the meantime I am also going to look for a pinch hitter editor to support this review to keep things moving forward.

everyone - please let me know how this sounds / if you have questions, concerns, etc / if i missed something!! ✨

thank you all for supporting our peer review process.

eliotwrobson commented 4 months ago

@lwasser thanks for checking in!! I think your assessment of the state of this review is correct. We've been trying to close all of the issues related to this review in a timely manner. There's one outstanding we may push back on somewhat given the effort involved and how long this review has been open, but otherwise there haven't been a ton of requests made by reviewers. With some support I think this can be pushed to close fairly soon (hopefully).

lwasser commented 4 months ago

ok great.

  1. you are welcome to push back and we can discuss . I think the importance of the fix relative to it's effort involved is more important than how long the review takes. As our goal is really that the package works, is usable etc if that makes sense. But we also want to ensure the reviews aren't overly imposing on an author / maintainer! so let's discuss.
  2. we are looking for another editor to help get this review over the finish line
  3. it will be important for @irisdyoung to submit a formal review here. because as it stands, when looking at the review it appears to only have one review (while i do understand issues have been opened). we do need that review template filled out so we have a formal record of two reviews! 👐🏻

we will get things wrapped up here @eliotwrobson let's work together on getting all three of the above elements sorted out!

eliotwrobson commented 4 months ago

In full agreement on 2 and 3. As for 1, in a previous comment, @phildong mentioned that he would be fine signing off his review once all the linked issues / PRs were closed: https://github.com/pyOpenSci/software-submission/issues/152#issuecomment-2111216359

There is only one outstanding item from this review, and this is a request for instructional examples using the package. We're working on this currently (as we have actually wanted to add something self-contained like this for a while), and we'll definitely have this added at some point, but my only pushback is whether this should be an item blocking acceptance. Since we're still waiting on other things, this isn't really an issue yet, but something to keep in mind for later on.

lwasser commented 4 months ago

ahhh ok i hear you @eliotwrobson !

so let me make a small list of todos' for this issue here - Eliot - can you fill in any gaps for me please? i haven't been able to dig deeply into this review but do want to help it move forward

I also see that for someone who doesn't know what automata is, it would be hard to follow that vignette and actually understand what it was doing. I wonder if we could discuss this in our slack with the community - the question here is about how rigorous docs should be when we accept vs if there is something that is good enough for now - and we know the authors are actively working on this. What do you all think?

i think we can discuss the first check above together. either here / on more synchronously in our slack. let me know what you all prefer. in the meantime i am going to pose the question in slack as it's really a standards related question for our review.

eliotwrobson commented 4 months ago

@lwasser thanks! discussing on slack is probably easier, I'm not on there currently but you can send me an invite at eliot.robson24@gmail.com!

Regarding the docs link, that seems like a good change, so I'll make a PR for that as soon as I get the chance 👍

Edit: actually, I think the repo link on pypi should stay as-is? The docs link there is correct, and the repo includes a docs link as well.

Edit 2: ahh I see what you mean now. Will discuss with the other maintainer.

Edit 3: Github repo link has been updated!

NimaSarajpoor commented 4 months ago

@eliotwrobson
I noticed an issue was already opened to enhance the examples. I checked the changes made to docs/examples/fa-examples.md. Great improvement! I am going to provide my comments just to make sure we have all things in one place. Btw, your package might be useful for engineers as well as they can better understand the concept of state / transition.

Please feel free to ignore if a similar comment was made by someone else before, and was already addressed / discussed. Also, these are just my perspective. So, feel free to let me know when you think a comment is not a good fit.

(1) Maybe add a link to examples on README page (Or maybe just provide one example on the README page) ~(2) If interested in getting more citations, you may want to add a citing part on README~ (3) Adding diagram (which I can see that it is addressed by calling the method show_diagram() in this opened issue) (4) Adding (printing) the output of read_user_input(my_dfa) can help user connect the dots (particularly, if they are trying to learn this concept, seeing the output may give them the "Eureka moment!") (5) Add the description of non-private methods to the doctoring of class DFA (I haven't checked other classes)

I do not see any other concerns from my end!

eliotwrobson commented 4 months ago

@NimaSarajpoor Thank you for the feedback! I think we should be able to add most of those items on to the open pull request you linked to.

The only one I had questions on was 5. Would this just be listing every public method in the top docstring for the DFA class? I'm not sure how valuable this would be in the scheme of things, since there's a huge number of methods, and these are all listed on the documentation site anyway. However, I'm not sure what common practice is in other libraries.

lwasser commented 4 months ago

Hi there - i'm just checking in to see how things are going 👋🏻 it looks like there are a few wrapup documentation elements to complete and then we can move this forward. @NimaSarajpoor THANK YOU for pitching in here!! i'll also add you as a reviewer to the header.

finally i am curious about the description being requested - i do see this

https://caleb531.github.io/automata/api/fa/class-dfa/

in the api docs and it looks like methods are generally documented. We can discuss more however as there might be a documentation piece that i'm missing as well!

Have a great weekend y'all. i'll check back in next week and we can work together to wrap this review up!

NimaSarajpoor commented 4 months ago

TL;DR Probably better to ignore (5)

@lwasser

finally i am curious about the description being requested - i do see this

You are absolutely right that the description was provided in the API docs. My comment was mainly about having the methods in the doctoring of the class

https://github.com/caleb531/automata/blob/530016a03703e071b72f1665769770039018536d/automata/fa/dfa.py#L54

@eliotwrobson

The only one I had questions on was 5. Would this just be listing every public method in the top docstring for the DFA class?

However, I'm not sure what common practice is in other libraries.

After you ask the question, I realized I was not sure myself! 😅 I did some research and found this stackoverflow post in which the answer quotes the following sentence from PEP 257:

The docstring for a class should summarize its behaviour and list the public methods and instance variables.

Still, the author of the answer mentioned that they prefer to avoid doing this because it can become out of date in the future.

eliotwrobson commented 4 months ago

@lwasser thanks for checking in! We just merged https://github.com/caleb531/automata/pull/227, which finishes addressing @/NimaSarajpoor's comments and the last open PR from this review (closing because our examples are now more extensive, and more comprehensive tutorials will be coming later). So I think we've addressed every open issue that's been raised thus far.

lwasser commented 4 months ago

ok y'all. It seems to me that in the latest PR all of the documentation issues have been addressed. We have one review formally submitted to this review from @phildong - thank you!! @phildong do you have content with the state of the package now after your review? Please just reply here if you have a moment.

@NimaSarajpoor i think from your statement above that you are ok with the state of the package docs (which was your focus here). can you please confirm by responding here?

NOTE: I did start a conversation on slack around that PEP. That seems fairly extensive - especially given we have tools to generate API docs and that pep was from 2001. BUT there could be other reasons for it too that I am missing (such as for those running help()). regardless if you can confirm that the changes made seem adequate that would be great.

@irisdyoung i just hope things are ok with you. I certainly understand hectic lives. I hope you are well! Please know that we never want to pressure / stress reviewers and volunteers at pyOpenSci and only wish for the best for everyone! Sometimes life gets busy and that is ok / (in fact recommended!!) to take care of yourself first!

NimaSarajpoor commented 4 months ago

@lwasser Yes. The docs look good to me. I confirm that there is no further concern from my end.

lwasser commented 4 months ago

thank you @NimaSarajpoor ok all we need is the ok from @phildong !!

phildong commented 4 months ago

oops sorry for the holdup on my side. I looked over the PR/conversations and everything looks nice! I've updated my original comment and I confirm there is no further concern from my side. One small thing: when I go to https://caleb531.github.io/automata/examples/fa-examples/ The updates in the examples doesn't seem to appear yet. But I'm assuming it will fix itself once the dev branch made to main/release, so everything is good in that case.

eliotwrobson commented 4 months ago

@phildong thanks for following up! This is indeed the case, the new version of the docs site will go up on a new deployment. You can see a preview of the page by looking at the branch: https://github.com/caleb531/automata/blob/develop/docs%2Fexamples%2Ffa-examples.md

sneakers-the-rat commented 4 months ago

Alright, sorry for my absence, am back at work now and Leah has asked me to finish up the review. It looks like we're all set here! we have two reviewers giving their all clear and all requested changes are made.

Here's the final approval checklist: https://www.pyopensci.org/software-peer-review/how-to/editors-guide.html#how-to-accept-a-package-into-the-pyopensci-ecosystem

The last substantive thing is to create an archive on zenodo - for the sake of the review you can just make a single archive of the repo (linked to a tagged commit, ideally), but you can also set up zenodo to automatically archive releases as well (sorry but these are the best docs i can find on zenodo itself, but if you search you'll find a guide). Once you do that, post the DOI, i'll add it to the top of the issue, and that should be that! If you'd like to submit a blog post as well, please let me know and i'll figure out where you should send that :)

Reviewers: you are welcome to join the pyopensci slack as well, I believe you should all either already be in there or you know how to contact me privately and i'll send you an invite!

thank you all again!

🎉 automata has been approved by pyOpenSci! Thank you @eliotwrobson for submitting automata and many thanks to @phildong, @NimaSarajpoor, and @irisdyoung for reviewing this package! 😸

Author Wrap Up Tasks

There are a few things left to do to wrap up this submission:

Editor Final Checks

Please complete the final steps to wrap up this review. Editor, please do the following:

If you have any feedback for us about the review process please feel free to share it here. We are always looking to improve our process and documentation in the peer-review-guide.

eliotwrobson commented 4 months ago

@sneakers-the-rat thank you! We've already set up automatic archiving through zenodo already: https://zenodo.org/records/10864492

I think it would be worthwhile to make a blog post as well, since we can just use examples / material from the JOSS paper or existing examples.

The revisions from items here will be included on Zenodo once we do another release. @caleb531 I've added the pyOpenSci badge to the develop branch along with the other changes, so we should be good to go. Would you like me to write the release notes? (can follow-up on slack if more discussion is needed).

sneakers-the-rat commented 4 months ago

great! added the DOI at the top, let me know when you make that release and that should be all!

caleb531 commented 4 months ago

@sneakers-the-rat Perfect! I've just published Automata v8.4.0 with all of the discussed changes and documentation improvements!

https://github.com/caleb531/automata/releases/tag/v8.4.0 https://pypi.org/project/automata-lib/8.4.0/ https://zenodo.org/records/12594180

cc @eliotwrobson

sneakers-the-rat commented 4 months ago

Excellent. Ill finalize whats left to do here and check with EiC for anything else that needs to happen. I am supposed to remind the authors and reviewers complete this post review survey to improve our processes if they would be so kind :)

Thanks and congratulations to everyone!!

eliotwrobson commented 4 months ago

@sneakers-the-rat excellent! May we write a blog post for the package? If so, I'll get to work on that pretty soon.