Skip to content

Conversation

@atravitz
Copy link
Contributor

while we're addressing this in the gufe docs overhaul, I thought I'd update in openfe docs as well.

Checklist

  • Added a news entry

Developers certificate of origin

@atravitz atravitz requested a review from mikemhenry March 13, 2025 18:31
@codecov
Copy link

codecov bot commented Mar 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.51%. Comparing base (c5d0e58) to head (749fee3).
Report is 94 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1190      +/-   ##
==========================================
- Coverage   94.15%   92.51%   -1.65%     
==========================================
  Files         141      141              
  Lines       10588    10588              
==========================================
- Hits         9969     9795     -174     
- Misses        619      793     +174     
Flag Coverage Δ
fast-tests 92.51% <ø> (?)
slow-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mikemhenry
Copy link
Contributor

Is this settled? Feels like we need a team wide discussion for this, it can be async. @jthorton @hannahbaumann see OpenFreeEnergy/gufe#508

Perhaps without opening the whole can of worms with naming things, I think really the only change in the PR that really changes the "name" is The GUFE package -> Gufe since the other changes I agree with even if it is an acronym (I like lowercase stylistically for software packages). Maybe we could go with gufe in that spot?

This is a minor preference, I am okay with Gufe for now in that spot until we decide what we want to do name wise.

@atravitz
Copy link
Contributor Author

Is this settled? Feels like we need a team wide discussion for this, it can be async. @jthorton @hannahbaumann see OpenFreeEnergy/gufe#508

I thought this was settled (see "visual style" here)

But if it's just for the package name, that' s fine by me.

@mikemhenry
Copy link
Contributor

AH--sorry, I thought this was motivated with acronym discussion -- my bad!

I would argue then that it should be gufe even if it is the start of a sentence (that is what the visual style says IMHO).

.. note::
We have reproduced API documentation from the `GUFE`_ package here for convenience.
The GUFE package serves as a foundation layer for openfe, providing abstract base classes and object models, and so might be more useful for developers.
We have reproduced API documentation from the `gufe`_ package here for convenience.
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably excessive but we can use a macro to not have to copy and paste this note in two places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

once we're >2 then I'll bother with a macro?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds like a good metric, see:
https://xkcd.com/1319/
https://xkcd.com/1205/

@atravitz
Copy link
Contributor Author

I would argue then that it should be gufe even if it is the start of a sentence (that is what the visual style says IMHO).

thanks! the start of sentence thing was ambiguous to me - I like keeping it lowercase.

@mikemhenry
Copy link
Contributor

I agree it is ambiguous and not clearly stated in the spec, so I would say there is an element of interpretation to it. We've got 2 votes now for gufe to be lower case at the start of a sentence. I'll post a comment on the discussion to see if we can get more consensus on it (I am also fine with merging this in)

atravitz and others added 2 commits March 14, 2025 08:28
Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
@github-actions
Copy link

No API break detected ✅

@atravitz atravitz merged commit 31bb2a7 into main Mar 14, 2025
13 checks passed
@atravitz atravitz deleted the docs/lowercase_gufe branch March 14, 2025 16:55
@dotsdl
Copy link
Member

dotsdl commented Mar 17, 2025

I'm partial to lowercase or lowercase for software packages. Thank you for making this change!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants