Repository navigation
Feat: add text associated with astropy partnership - #225
Conversation
|
the failure is only because html proofer is looking for this page online and it doesn't exist - yet. otherwise all systems are a go! 🚀 |
|
Potentially of interest to @jibarnum, @sapols, and @rebeccaringuette from PyHC! |
Related discussionsThis is a PR related to what the partner pages might look like And this issue is a discussion of metrics related to package health that we might want to track over time. |
namurphy
left a comment
namurphy
left a comment
There was a problem hiding this comment.
Had a few pretty minor suggestions. Overall this is sounding pretty great!
| About Our Partnerships <self> | ||
| JOSS <joss> | ||
| Astropy <astropy> | ||
| Pangeo <pangeo> |
There was a problem hiding this comment.
@namurphy i'm excited about it too!! we also just spoke with scverse!!
| - Reviewed against current pyOpenSci guidelines and checks. | ||
| - Also reviewed against the Astropy-specific guidelines listed below. | ||
|
|
||
| (astropy-collaboration)= |
There was a problem hiding this comment.
I didn't know you could do this in Markdown. Cool!
There was a problem hiding this comment.
it might be a superpower of MyST . i really love MyST for all of our documentation and anything sphinx related. it's such a game changer in terms of flexibility / readability with simple syntax!
There was a problem hiding this comment.
What is (astropy-collaboration)=? What should I be expected to see with this directive in the rendered page?
There was a problem hiding this comment.
oh @pllim that is a myst target. it's a nice way to have a consistent target link in case someone changes header text and forgets and also to just make a nice short target link. so you can think of it as a shortcut to make cross linking to section headers within a sphinx document less error prone. i'm not sure it's visible in the final rendered page however just internally it's helpful in case people change header names.
| :::{figure-md} astropy-logo | ||
| :class: myclass | ||
|
|
||
| <img src="/images/astropy_project_logo.svg" alt="The astropy logo. The logo is an orange egg shape with a white snake spiraling in it. The text of the image says The Astropy Project on three lines." class="mb-1" width="400px"> |
There was a problem hiding this comment.
Yay for alt-text & image descriptions!
There was a problem hiding this comment.
Trying hard to ensure we have an accessible guide @namurphy !! i'm so glad you appreciate it!!!
|
I think this captures Astropy's extra requirements well! |
|
I have it in my notes that @eteq would very much want to review this but his response might be delayed. |
|
@all-contributors please add @pllim for code, review ok no problem @eteq - looking forward to your input here. in the meantime i'll add y'all as contributors to our guide here!! |
|
I've put up a pull request to add @pllim! 🎉 |
|
this is failing because of hte same htmlproofer issue mentioned here - pyOpenSci/pyopensci.github.io#241 which will be fixed at some point in the future! |
pllim
left a comment
pllim
left a comment
There was a problem hiding this comment.
On a high level, this LGTM but I would like @WilliamJamieson and @dhomeier to fine tune the details, if necessary, because they are the current Editors from Astropy.
If you ever fixed the rendered HTML preview, I would love to see that before merge as well.
Thank you!
|
@pllim you can always view the rendered docs on circle ci here is this what you mean? it should update with every change to this pr as well so we can see what it looks like prior to merging. below is a screenshot from circle ci preview 
|
|
what i'd love to do in the future is work a bit on a nice graphic for our partnerships. when i do that i'll run ideas by everyone as well. |
bb4222f to
df1ff25
Compare
pllim
left a comment
pllim
left a comment
There was a problem hiding this comment.
Minor comments, mostly with how we brand astropy (just the core library) vs Astropy as a project.
As for the integration rules, might have to fine tune when Astropy get to updating https://github.com/astropy/astropy-project/blob/main/affiliated/ contents but seems fine for now as written (i.e., does not conflict with what we already do over at Astropy but seems repetitive).
| - Reviewed against current pyOpenSci guidelines and checks. | ||
| - Also reviewed against the Astropy-specific guidelines listed below. | ||
|
|
||
| (astropy-collaboration)= |
There was a problem hiding this comment.
What is (astropy-collaboration)=? What should I be expected to see with this directive in the rendered page?
|
p.s. Ah, my bad, I didn't realize rendered page is a different entry in the CI listing. I can see it here: https://output.circle-artifacts.com/output/job/1cb7525e-e3e6-4a4a-bec7-97aec45e29da/artifacts/0/html/partners/astropy.html I think you need a logo that can adapt to dark mode? 
We have one here: https://github.com/astropy/astropy/blob/main/docs/_static/astropy_banner_96_dark.png (if you don't want that one, check out others in https://github.com/astropy/astropy/tree/main/docs/_static ) Thanks! |
|
ok i've integrated all of your edits @pllim . i understand this feels repetitive. however, for now i'd like to have overlapping language here and in astropy only because we can then internally link in our review template to this page in the guidebook for consistency and also ensure our partnerships are consistent. if we find in the future that dedicating an entire page is over doing it / difficult to co-maintain we can always revisit. but i find these pages are also useful for new partners to look at to see how we're leveraging the review process and working together. what do you think the next step is in terms of enough approval for us to merge this? |
|
NOTE: i have to update the logo to include the dark mode adapted one! and then we can potentially merge this is astropy folks are on board with that.
|
|
hey @pllim et al. so the image on that page is just a markdown image - its not managed with styles. as such i really don't know how to implement an astropy logo that works on both dark and light mode without creating something new. our book theme has both dark and light mode so the logo will need to look good on both. right now the one with the white background is the only one that renders properly. the others are either for dark or light mode but i'm not sure both. :::{figure-md} astropy-logo
:class: myclass
<img src="/images/astropy_project_logo.svg" alt="The astropy logo. The logo is an orange egg shape with a white snake spiraling in it. The text of the image says The Astropy Project on three lines." class="mb-1" width="400px">
:::below is what things look like in dark vs light mode 
i'm happy to change the image but i'd need someone to provide me with an image that works in both cases OR if you know how to manage dark and light mode within a markdown image call that still has alt text, etc that could work too? otherwise we'll have to stick to the one with the white background. |
1d7a0bc to
d93942d
Compare
|
i just noticed i am not linking to your affiliated packages language on the Astropy website. so i just pushed a small commit ensuring that link as well. I also fixed case - capital A when discussing the project / community / lower case for the package. |
|
Thanks for the cross-link! It is okay if you can't get dark mode logo to work; not the end of the world. Thanks for trying! |
| If you are submitting a package for review and wish to also apply for Astropy affiliation, your package will be: | ||
|
|
||
| * Reviewed against current pyOpenSci guidelines and checks. | ||
| * Also reviewed against the Astropy-specific guidelines listed below. | ||
|
|
||
| If you are submitting a package for review and wish to also apply for Astropy affiliation, your package will be: | ||
|
|
||
| - Reviewed against current pyOpenSci guidelines and checks. | ||
| - Also reviewed against the Astropy-specific guidelines listed below. |
There was a problem hiding this comment.
This text is duplicated, just with different symbols ("-" vs "*").
There was a problem hiding this comment.
oh gosh - good catch. i'll fix that.
There was a problem hiding this comment.
that must have been a merge conflict fix gone wrong is my only guess 😆
thanks @pllim yes i'm just not sure how to deal that. i'm not a fan of these themes with light and dark mode as they are so much harder to maintain well. Things like this come up a lot! i wonder if i can force the theme to remove dark mode altogether |
|
|
||
| ## Long-term maintenance & affiliate status over time | ||
|
|
||
| All packages accepted into the pyOpenSci ecosystem will follow the [pyOpenSci |
|
My approval stands. @dhomeier , would be nice if you can approve too. Thanks, all! |
|
ok y'all. It's march 1 - merge day! merging this!! |
|
👏 👏 👏 |
|
Sorry, too much end-of-month business around here; looks great with all those review eyes gone over it. |
|
@dhomeier , this is already merged. Maybe submit your suggestion as a PR? Thanks! |
This is a draft of the landing page for the astropy partnership in our peer review guide. This page specifies the requirements for a package to become astropy-affiliated.
it is based on this document in which i took all of the affiliated requirements that i could find from the astropy website / github and tried to match them with our default requirements. i then pulled out the astropy specific elements. Any and all feedback is welcome on this pr.
NOTE: i'm going to also loop in the pyhc / sunpy / plasmapy folks on this - just so they can see what the process looks like.
cc: @dstansby @namurphy can you kindly tag anyone else who might want to see this process please? i am not yet connected with all of the communities here on github.