Skip to content
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

Documentation: Rehab the Reference Page #2220

Closed
3 of 5 tasks
bms63 opened this issue Nov 6, 2023 · 15 comments · Fixed by #2272
Closed
3 of 5 tasks

Documentation: Rehab the Reference Page #2220

bms63 opened this issue Nov 6, 2023 · 15 comments · Fixed by #2272
Labels
documentation Improvements or additions to documentation function re-work issues identified by function re-work gang - usually argument alignment, user friendliness

Comments

@bms63
Copy link
Collaborator

bms63 commented Nov 6, 2023

Please select a category the issue is focused on?

Function Documentation

Let us know where something needs a refresh or put your idea here!

As we push to 1.0 we are trying to reduce the number of functions visible in our documentation that are not helpful for users.

For theses eleven functions: 8 will be completely removed and 3 will be exported but have keywords/family changed to internal.

  • Remove: We no longer should export this functions and so @export is removed and keywords and family removed
  • Export, but change keyword to internal: These functions should still be exported, but their keyword and family changed to internal so they no longer appear on our reference page.

assert_db_requirements() - Remove
assert_terms() - Remove
assert_valid_queries() - Remove
extend_source_datasets() - Remove
filter_date_sources() - Remove
format(<basket_select>) - Export, but change keyword to internal
validate_basket_select() - Remove
validate_query() - Remove
chr2vars() - Remove
signal_duplicate_records() - Export, but change keywords to internal
extract_duplicate_records() - Export, but change keywords to internal

Definition of Done

  • All 11 functions documentation updated
  • Check vignettes and templates are NOT using the 11 functions
  • Update to News to list out what we have removed/suppressed - no need to do any deprecation process.
  • Check that no extension packages might be using these 11 functions.
  • Proceed with normal PR.

Hi @pharmaverse/admiral another nice one!!

@bms63 bms63 added documentation Improvements or additions to documentation function re-work issues identified by function re-work gang - usually argument alignment, user friendliness labels Nov 6, 2023
@bundfussr
Copy link
Collaborator

@bms63 , should we create an issue in admiraldev to update the programming strategy?

@bms63
Copy link
Collaborator Author

bms63 commented Nov 7, 2023

Yes! Good call. I will make an issue

@bms63
Copy link
Collaborator Author

bms63 commented Nov 7, 2023

@bms63 , should we create an issue in admiraldev to update the programming strategy?

@bundfussr pharmaverse/admiraldev#346

@sophie-gem
Copy link
Collaborator

I can pick this up and do this tomorrow morning.

@sophie-gem sophie-gem self-assigned this Nov 15, 2023
sophie-gem added a commit that referenced this issue Nov 15, 2023
…ternally. (Full list available in issue definition.
@sophie-gem
Copy link
Collaborator

Note to self: 4 templates (from ADPP) and vignettes left to check.

@sophie-gem
Copy link
Collaborator

Hi @bms63 , @bundfussr , in terms of checking the admiral extension packages for this...have I remembered all below:

  • admiraloptha
  • admiralonco
  • admiralvaccine

Also, is anyone able to check your company specific admiral packages: admiralroche, admiralgsk, etc?

@bundfussr
Copy link
Collaborator

Hi @bms63 , @bundfussr , in terms of checking the admiral extension packages for this...have I remembered all below:

  • admiraloptha
  • admiralonco
  • admiralvaccine

Also, is anyone able to check your company specific admiral packages: admiralroche, admiralgsk, etc?

@sophie-gem , you remembered the extensions packages correctly.

I have checked admiralroche. It uses chr2vars() and signal_duplicates_records().

@bms63
Copy link
Collaborator Author

bms63 commented Nov 17, 2023

Hi @bms63 , @bundfussr , in terms of checking the admiral extension packages for this...have I remembered all below:

  • admiraloptha
  • admiralonco
  • admiralvaccine

Also, is anyone able to check your company specific admiral packages: admiralroche, admiralgsk, etc?

@sophie-gem , you remembered the extensions packages correctly.

I have checked admiralroche. It uses chr2vars() and signal_duplicates_records().

Then we should still export chr2vars but use internal? @bundfussr

@bundfussr
Copy link
Collaborator

Hi @bms63 , @bundfussr , in terms of checking the admiral extension packages for this...have I remembered all below:

  • admiraloptha
  • admiralonco
  • admiralvaccine

Also, is anyone able to check your company specific admiral packages: admiralroche, admiralgsk, etc?

@sophie-gem , you remembered the extensions packages correctly.
I have checked admiralroche. It uses chr2vars() and signal_duplicates_records().

Then we should still export chr2vars but use internal? @bundfussr

I would export both and also list them on the reference page.

@sophie-gem
Copy link
Collaborator

So given what @bundfussr said above, I will retract the changes to chr2vars() and signal_duplicates_records() and check the 3 admiral extensions (R functions, templates, and vignettes) for any of these functions.

@bms63
Copy link
Collaborator Author

bms63 commented Nov 21, 2023

Awesome thanks @sophie-gem !!

@sophie-gem
Copy link
Collaborator

Note to self: No templates contain these functions. Rest of vignettes left to check from bds_finding onwards.

@bms63
Copy link
Collaborator Author

bms63 commented Nov 29, 2023

Hey @sophie-gem can you finish this by the end of the week?

@sophie-gem
Copy link
Collaborator

Ah, so sorry - dropped off the planet again! Yes, I will aim to get this done by EOB Friday. Is that ok?

@bms63
Copy link
Collaborator Author

bms63 commented Nov 29, 2023

Ah, so sorry - dropped off the planet again! Yes, I will aim to get this done by EOB Friday. Is that ok?

yes no problem!

@sophie-gem sophie-gem removed their assignment Nov 30, 2023
bms63 added a commit that referenced this issue Dec 1, 2023
bms63 added a commit that referenced this issue Dec 1, 2023
@bms63 bms63 linked a pull request Dec 1, 2023 that will close this issue
15 tasks
bms63 added a commit that referenced this issue Dec 1, 2023
bms63 added a commit that referenced this issue Dec 1, 2023
bms63 added a commit that referenced this issue Dec 1, 2023
* #2220 - 8 functions no longer exported, 3 functions exported - but internally. (Full list available in issue definition.

* chore: #2220 removing functions from docs, others to internal

* chore: #2220 putting back some keywords/family

* docs: news entry, removing see also references

* fix: re-exporting chr2vars.  removing reference in derive_vars_query

* chore: #2220 spelling

* chore: #2220 styling - don't i look good!!

* fix: #2220 Note in R-CMD check

* docs: #2220 a dangling roxygen comment

---------

Co-authored-by: Sophie Shapcott <sophie.shapcott@gemprogramming.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation function re-work issues identified by function re-work gang - usually argument alignment, user friendliness
Development

Successfully merging a pull request may close this issue.

3 participants