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

Add function documentation #97

Merged
merged 9 commits into from
Oct 29, 2024
Merged

Add function documentation #97

merged 9 commits into from
Oct 29, 2024

Conversation

awasyn
Copy link
Collaborator

@awasyn awasyn commented Oct 10, 2024

Document parameters

What kind of change(s) are included?

Checklist

Please ensure that all boxes are checked before indicating that this pull request is ready for review.

  • I have read and followed the CONTRIBUTING.md guidelines.
  • I have searched for existing content to ensure this is not a duplicate.
  • I have performed a self-review of these additions (including spelling, grammar, and related).
  • I have added comments to my code to help provide understanding.
  • I have added a test which covers the code changes found within this PR.
  • I have deleted all non-relevant text in this pull request template.
  • Reviewer assignment: Tag a relevant team member to review and approve the changes.

@the-mayer

Signed-off-by: Awa Synthia <[email protected]>
@awasyn awasyn requested a review from the-mayer October 10, 2024 22:46
Signed-off-by: Awa Synthia <[email protected]>
@the-mayer the-mayer self-assigned this Oct 11, 2024
@the-mayer the-mayer added the documentation Improvements or additions to documentation, incl. R docstring/roxygen2 label Oct 11, 2024
@jananiravi jananiravi added the outreachy for outreachy interns label Oct 20, 2024
Comment on lines 90 to 93
#' # Example genomic context data frame
#' prot <- data.frame(GenContext = c("A>B", "C<D", "E=F*G", "H>I"))
#' reversed_prot <- reverseOperonSeq(prot)
#' print(reversed_prot)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@awasyn , can you double check this example code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. I will run example again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still seeing an R-CMD check error when running this example:

❯ checking examples ... [14s/15s] ERROR
  Running examples in ‘MolEvolvR-Ex.R’ failed
  The error most likely occurred in:
  
  > base::assign(".ptime", proc.time(), pos = "CheckExEnv")
  > ### Name: reverseOperonSeq
  > ### Title: reverseOperon: Reverse the Direction of Operons in Genomic
  > ###   ContextSeq
  > ### Aliases: reverseOperonSeq
  > 
  > ### ** Examples
  > 
  > # Example genomic context data frame
  > prot <- data.frame(GenContext = c("A>B", "C<D", "E=F*G", "H>I"))
  > reversed_prot <- reverseOperonSeq(prot)
  Error in ge[[x]] : subscript out of bounds
  Calls: reverseOperonSeq -> lapply -> FUN -> straightenOperonSeq
  Execution halted

Using the example data defined in prot the error occurs during the lapply operation @line 137 as ge has length 0 due to the previous subset operation.

R/CHANGED-pre-msa-tree.R Outdated Show resolved Hide resolved
R/ipr2viz.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@the-mayer the-mayer left a comment

Choose a reason for hiding this comment

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

Excellent work adding to the package documentation! I've made a few requests for changes (picked some representative examples, but changes should be made throughout).

Overall this looks great and is another step towards enabling the CI in #34

Copy link
Collaborator

@the-mayer the-mayer left a comment

Choose a reason for hiding this comment

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

I've disabled the reverseOperonSeq() example for now and converted to a new issue so that other changes may be merged. Thanks @awasyn !

@the-mayer the-mayer merged commit bae52b0 into JRaviLab:main Oct 29, 2024
1 check passed
@awasyn
Copy link
Collaborator Author

awasyn commented Oct 30, 2024

Thanks @the-mayer , sorry for the delay. I was finishing my final application for the internship.
However i have recently tried to pass different examples and g[[x]] is empty every time. I'll add this clean up to my milestones. Error handling and refactoring the function..

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, incl. R docstring/roxygen2 outreachy for outreachy interns
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants