Skip to content

Fix: Update scope document and also fix a few sphinx bugs #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 32 commits into from
Feb 21, 2023

Conversation

lwasser
Copy link
Member

@lwasser lwasser commented Jan 31, 2023

This pr does a few things

  • It updates the scope document to be a bit more clear and specific addressing Current domain/scope for Python packages on pyOpenSci #152 in part and also comments by @eriknw comments - erik if you don't mind i'd love for you to look at the changes here.
  • it also fixes a few quirks in the pydata theme that were happening with the header.
    I would love to merge this by 9 feb 2023 if possible.

@@ -18,8 +18,6 @@ Currently, the packages that pyOpenSci reviews also need to fall into the
technical and applied scope of our organization. This scope may expand over time
as the organization grows.



## Is Your Package in Scope For pyOpenSci Review?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eriknw @NickleDave @arianesasso @cmarmo @sneakers-the-rat @Batalex i'd LOVE your input on these changes. this page is really the only one you have to look at in this review. the rest of the changes are just link fixes and cleanup.

Many thanks in advance for this!

Copy link

@eriknw eriknw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a very quick read-through, and I think this looks great!

lwasser and others added 3 commits February 1, 2023 10:18
@lwasser
Copy link
Member Author

lwasser commented Feb 1, 2023

TODO: There are still two COC broken links:

External link https://www.pyopensci.org/software-peer-review/CODE_OF_CONDUCT.html failed (status code 404)

all COC links should go to governance/CODE_OF_CONDUCT.html now

may not be in-scope today. We strive for consistency in our peer review process. However, we also evaluate packages on a case-by-case basis.
In some cases exceptions are made.
```

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: i think we should add a policy around telemetry in packages. a current review has it. i think we should discourage against it but if it's required make it opt in rather than opt out preferred?

We should also evaluate whether we want to review general data manipulation and coding tools. ROS does not review these tools. but defining what that means is tricky.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps start with a narrower scope? It's easy to broaden it later, but not the other way around.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point! is this a specific comment related to our scope as defined now or just to that statement above? I added that given Ropensci has similar language and it just allows us to adjust in the future in case we realize we are headed down a rabbit hole that doesn't work for us! (kind of like a legal disclaimer :) in case we need to modify scope)

We've had the same scope for the past 4 years BUT of course we are being more specific now about each category given all of the questions we've gotten :)


### Data processing & munging

Data munging tools are tools that support processing data discussed above. This
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth giving a definition of munging here, or linking to one? (like https://en.wikipedia.org/wiki/Data_wrangling) Something like "transforming data in a way that makes further analysis possible" (e.g. Physcraper updating local phylogenies with publicly available data)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

absolutely - want to fix inline please?

considering whether open-source options exist.

<!-- TODO: need an example for this category -->
* Examples: We don't have a package in this category yet - *Could be your package?*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't pyGMT an example? Because it wraps GMT.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think so. pyGMT actually fits into several of our categories i think i just placed it in one. but if we implement your suggestion above then we demonstrate that a package can be in several categories. i'm fine with that!

@lwasser
Copy link
Member Author

lwasser commented Feb 14, 2023

@all-contributors please add @yuvipanda for code, design, review

@allcontributors
Copy link
Contributor

@lwasser

I've put up a pull request to add @yuvipanda! 🎉

@lwasser
Copy link
Member Author

lwasser commented Feb 14, 2023

✏️ second round of feedback is welcome on this PR by 20 feb 2023 at midnight mountain time (for those in other time zones that will be in the morning for ya!). i'll plan to merge around 21 feb 2023 (depending on how long it takes to work through edits!). Many thanks y'all. i think our policies are coming together nicely.

data lifecycle. Packages submitted to pyOpenSci should fit into one or
more of the categories below and should be within our technical scope.

```{admonition} Package Use Metrics Are Not a Requirement for Review
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggests "we don't need you to go and measure your package use (but we may consider how much it is used)". Rather: "Widespread package usage is not required for review", "We review independent of popularity", or similar?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many thanks @stefanv fixing. @NickleDave i think had similar feedback here and i haven't been able to get it right!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this any better :

Your Package Does Not Need to Have Widespread Use to be Reviewed
:class: important

We review packages across a spectrum of small to large user bases. The popularity of your package is not a consideration in our review process!

@NickleDave

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me!

nit: would maybe say it like "Your Package Does Not Need to Be In Widespread Use Yet to be Reviewed" or "Your Package Does Not Need to Be Widely Used to be Reviewed"? ("in use" not "have use")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was merged a few hours ago... would you be open to creating a PR for this - i'm fine with cleaner wording.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, sorry, was catching up with replies

Comment on lines 47 to 48
As such, the number of GitHub stars or PyPI / Conda
downloads is NOT considered as a part of our scope evaluation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very explicit, but unsure it adds much content.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're worried about users being confused at this point already, perhaps it's worth clarifying WHY you are reviewing in the first place. It's not to establish how useful a package is, but how well it is put together. This is a subtle change of emphasis on the "we only consider" clause above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok and now i'm here -

We review packages with the goal of improving package quality and usability for scientists.
As such, we review packages across a spectrum of small to large user bases. The popularity of your package is not a consideration in our review process!

When we evaluate whether you package is within our scope, we only consider:

  1. how the package is developed and
  2. how the package relates to and supports the broader scientific ecosystem

We welcome young packages that are just entering the scientific Python
ecosystem to apply for review if they are relevant to the science community and
fit into at least one scope category below. We also welcome mature packages with
a growing or established community!

may not be in-scope today. We strive for consistency in our peer review process. However, we also evaluate packages on a case-by-case basis.
In some cases exceptions are made.
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps start with a narrower scope? It's easy to broaden it later, but not the other way around.


## Domain areas

In addition, our scope includes focused domain areas. These areas are based on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "computational library" class, described here, seems to be missing from the previous section, when it's arguably the first category included?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Say a little bit more about what you mean?
I think you're suggesting that there should be another category above that's like "general library or framework for a domain or research area"?
E.g. python-graphblas would be such a library (in addition to being a wrapper)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh... i missed a few comments here! i will make sure these are added in a separate PR. i am not sure how i missed this

Copy link
Contributor

@NickleDave NickleDave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggesting changes to category/domain sections as discussed in Slack 😸


## Domain areas

In addition, our scope includes focused domain areas. These areas are based on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Say a little bit more about what you mean?
I think you're suggesting that there should be another category above that's like "general library or framework for a domain or research area"?
E.g. python-graphblas would be such a library (in addition to being a wrapper)

```

### Telemetry & user-informed consent
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to pull this into it's own PR so we can link it explicitly to the open issues and discussion.

@lwasser
Copy link
Member Author

lwasser commented Feb 21, 2023

ok y'all... merging this and will open a NEW pr with telemetry in it. we can always edit this further as we need to - even in the other PR on telemetry when i open it!! many thanks for all of the feedback!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants