search

astropy / astropy-APEs

A repository storing the Astropy Proposals for Enhancement.
Other
35 stars 37 forks source link

APE 24: Astropy Affiliated Packages with pyOpenSci (now APE 22) #87

Closed pllim closed 9 months ago

pllim commented 1 year ago

Initial authors: @pllim @lwasser @WilliamJamieson @dhomeier

TODO

pllim commented 1 year ago

@lwasser @WilliamJamieson @dhomeier -- Please proof-read before I open this up to the rest of Astropy. Thank you!

lwasser commented 1 year ago

ok @pllim i added some feedback here. generally i think it looks great! this was a lot of work! please let me know if you need anything else.

lwasser commented 1 year ago

hey there @pllim 👋 just following up on our slack chat here! In the APE i think it would make sense to mention that the astropy editors and reviewers will need to adhere to our timeline so the package maintainer have a good experience with the review and things don't hang for long periods of time. Generally the editor role shouldn't take a huge amount of time BUT it's important for an editor, once the review starts, to check in on the review periodically - ever few weeks or so and more when it's wrapping up. this allows the maintainer to have a good start to end experience and avoids reviews dragging out for months to a year!

pllim commented 1 year ago

@lwasser , are you good with the text now? Thanks!

lwasser commented 1 year ago

@pllim i am good with this as is 🎉 !! i left a note about the two package guides - scientific python project vs pyopensci. so the main point of clarification is that we maintain our own community developed packaging guide so that is what we rely on and direct people to for peer review if they have questions - the scientific python dev guide is a separate document that is maintained by someone else. so i probably wouldn't add it to the ape surrounding peer review here with us specifically but i think it's a great resource that the astropy community should take advantage of.

i hope that helps. but again i support this moving forward / being merged via whatever process your community has in place!

let me know if you need anything else from me and my apologies for missing your replies to my comments above.

pllim commented 1 year ago

I have pinged the Editors multiple times. I think at least one of them are okay with reviewing this as part of the "public comments" period, so I took this out of draft. I will send out memo to astropy-dev.

dhomeier commented 1 year ago

Sorry, I did not mean that to block any comments on this – as @Cadair's comment above shows, more eyes to catch the possible side-effects are certainly asked for at this point.

lwasser commented 1 year ago

please ping me on any questions, concerns etc! happy to help here as much as i can.

pllim commented 12 months ago

make Astropy look bad

That was not my intention at all and I apologize if the effect is such. I was trying to describe the current situation in what I thought an objective way to provide motivation on why we decided to revamp the process. I was also an Editor, so the original text was from my own experience, plus what I think I saw after. I was not involved before that and could not provide an objective summary of the experience retroactively.

tl;dr -- I take full responsibility of the text you found offensive and I apologize. Thanks for the edits!

pllim commented 12 months ago

@hamogu , does the draft look acceptable to you now? If so, please clear your "changes requested" in the review. Thanks!

hamogu commented 12 months ago

tl;dr -- I take full responsibility of the text you found offensive and I apologize. Thanks for the edits!

Offensive is too strong a word. I would prefer "respectfully disagreed".

lwasser commented 11 months ago

I am fully in favor of joining pyOpenSci and replacing our current process with the PyOpenSci process.

I'm also convinced that we can work out any small questions (e.g. do we recommend the OpenAstronomy packaging or pyopensci or both?).

However, I'm very surprised by how negative this APE describes our current process for affiliated packages - I'm even more surprised because both our current affiliated edotors are listed as co-authors. I've left specific suggestion in places, but the general tone in this APE is to bad-mouth our existing process and I don't think that's helpful. This is an APE for changing a process, and we should fairly describe what's happening - pyOpenSci has a lot to offer without us writing this to make Astropy look bad - because I think it's not. The concept of affilated packages - that's been in place for a decade - has been very successful for the growth of our ecosystem.

Friends - i hope that you all know that i hold your existing review process with high respect. it's time consuming to build a review process and you've been developing / running this community run process for a solid 10 years! so i just wanted to re-inforce the fact that i hope our partnership supports astropy. Perhaps by leveraging our processes and community it will even create space for other work that you need to do as a community.

pllim commented 11 months ago

Sorry for the delay!

@dhomeier , I think I addressed your comments, except for one that I do not understand, which I have replied above.

How is this APE looking now? Anymore changes needed?

pllim commented 11 months ago

Thanks, everyone! I have accepted Derek's updated suggestion based on other feedback.

I am hoping that @WilliamJamieson can still have a look since he is the other Astropy Editor. But I guess this has been open long enough that CoCo can proceed to a final decision if William cannot get to this by the next CoCo meeting on 2023-12-18?

Also Derek said I should also ping @eteq to have a look, hopefully also by 2023-12-18.

pllim commented 10 months ago

CoCo met today (2023-12-18) and discussed this. Out of 5, 2 abstained because they are also authors of this APE, 2 approved, and 1 absent.

lwasser commented 10 months ago

CoCo met today (2023-12-18) and discussed this. Out of 5, 2 abstained because they are also authors of this APE, 2 approved, and 1 absent.

amazing news @pllim !!

lwasser commented 9 months ago

I also have one larger question that I'm sure we talked about so I'm confident it has been addressed but I couldn't find an explcit statement in this text: are we guaranteed, say at least one editor seat on pyopensci? I think it would be good to say that, not so much because I'm worried pyopensci wouldn't do it, but as a way to ensure we within Astropy stay plugged in.

Basically, I want to ensure we have a role that the CoCo feels they should try to keep at least one community member in so that we don't end up saying "well pyopensci has got it, so we can let that lapse". (Partly just because that doesn't seem fair to @lwasser et al!)

@eteq absolutely. I'm so glad you brought that up!

I think it is absolutely critical that astropy have atleast 1 person on the pyOpenSci editorial board and that we ensure that we also stay in touch / communicate. We could even consider a checkin once a year or at whatever cadence makes sense to ensure everyone is on the same page!!

eteq commented 9 months ago

Alright, I'm happy with this, as is the rest of the coco. Merging!

eteq commented 9 months ago

Oops, I was supposed to do the decision rationale, etc in this PR but I'll just do it directly in main instead. My bad.

pllim commented 9 months ago

Thanks, all!

eteq commented 9 months ago

Not also this was renumbered to APE22 in 6964140

pllim commented 9 months ago

Wait... why is the APE number changed?

pllim commented 9 months ago

APE 22 is already claimed but just not merged:

Any chance you can change this back to original APE 24 to avoid confusion?

pllim commented 9 months ago

Also there is a typo in the rationale: commitee -> committee

eteq commented 9 months ago

@lwasser - I think I found the right ORCiD for you in https://doi.org/10.5281/zenodo.10581891, but let me know if any of that is wrong and I can fix the metadata!

eteq commented 9 months ago

Any chance you can change this back to original APE 24 to avoid confusion?

The procedure in the README covers this @pllim - the numbering is in merging order. the numbering prior to merging is to be thought of as preliminary.

(The backstory is that we were struggling with deciding what to do here, and then we wrote it into the instructions and no one seemed to mind... So if we want to change that it's just a matter of changing the README instructions. I don't have a terribly strong opinion other than "we should make sure the instructions are explicit about it and follow whatever they say at any given time")

pllim commented 9 months ago

🤪

dhomeier commented 9 months ago

I suppose the final renaming is only done right in the merge commit then – any special tricks how to do this?