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

Update lib docs #5492

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

christopher-hakkaart
Copy link
Contributor

  • Add -lib in lib sharing section
  • Add Custom lib directory section to cli users section

cc @robsyme

Signed-off-by: Christopher Hakkaart <[email protected]>
@christopher-hakkaart christopher-hakkaart requested a review from a team as a code owner November 11, 2024 14:12
Copy link

netlify bot commented Nov 11, 2024

Deploy Preview for nextflow-docs-staging ready!

Name Link
🔨 Latest commit fe65e0a
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/67332e6cbf314500089a9e2e
😎 Deploy Preview https://deploy-preview-5492--nextflow-docs-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@robsyme
Copy link
Collaborator

robsyme commented Nov 11, 2024

I think that the lib directory wording could be more precise.

Any Groovy scripts or JAR files in the lib directory in the project directory root will be available inside the Nextflow workflow automatically.

The "Any Groovy scripts" feels a bit too loosely defined. The directory is added to the classpath, which means that any classes or packages defined therein will be available in the execution context. Loose scripts, or functions defined outside of classes will not be available to Nextflow.

You can load arbitrary assets in this lib directory with classLoader.getResource:

workflow {
    def myFile = this.class.classLoader.getResource("my-file.txt")
    println = myFile?.text
}

But imho, this should be discouraged. It should be used for loading classes and packages only, and the documentation should encourage this by making it explicit that the lib directory is to be used to make classes and packages available in the workflow.

@robsyme
Copy link
Collaborator

robsyme commented Nov 11, 2024

@pditommaso has also noted internally that the -lib CLI argument encourages bad practice and is likely going to be deprecated, so any documentation addition would be best focused on documenting the lib directory (which will be retained).

@christopher-hakkaart christopher-hakkaart marked this pull request as draft November 11, 2024 16:45
@christopher-hakkaart
Copy link
Contributor Author

All good. I've converted this PR to a draft and re-work it to focus on documenting best practises for the lib directory.

@robsyme
Copy link
Collaborator

robsyme commented Nov 11, 2024

Potential example:

// lib/DNASequence.groovy
class DNASequence {
    String sequence

    // Constructor
    DNASequence(String sequence) {
        this.sequence = sequence.toUpperCase() // Ensure sequence is in uppercase for consistency
    }

    // Method to calculate melting temperature using the Wallace rule
    double getMeltingTemperature() {
        int gCount = sequence.count('G')
        int cCount = sequence.count('C')
        int aCount = sequence.count('A')
        int tCount = sequence.count('T')

        // Wallace rule calculation
        double tm = 4 * (gCount + cCount) + 2 * (aCount + tCount)
        return tm
    }

    String toString() {
        return "DNA[$sequence]"
    }
}
// main.nf
workflow {
    Channel.of('ACGTTGCAATGCCGTA', 'GCGTACGGTACGTTAC')
    .map { seq -> new DNASequence(seq) }
    .view { dna -> 
        def meltTemp = dna.getMeltingTemperature()
        "Found sequence '$dna' with melting temperaure ${meltTemp}°C" 
    }
}

Which returns

Found sequence 'DNA[ACGTTGCAATGCCGTA]' with melting temperaure 48.0°C
Found sequence 'DNA[GCGTACGGTACGTTAC]' with melting temperaure 50.0°C

Signed-off-by: Christopher Hakkaart <[email protected]>
Signed-off-by: Christopher Hakkaart <[email protected]>
@christopher-hakkaart
Copy link
Contributor Author

Thanks for the example. I've updated the PR with the example and a revised lib section.

The sentence "Scripts or functions defined outside of classes will not be available in the execution context." could be made into a warning either before or after the example if you think it should be more prominent.

Comment on lines +136 to +138
int gCount = sequence.count('G')
int cCount = sequence.count('C')
int aCount = sequence.count('A')
Copy link
Member

Choose a reason for hiding this comment

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

but hard to read those variables maybe g_count or gc is a better alternative

Comment on lines +136 to +142
int gCount = sequence.count('G')
int cCount = sequence.count('C')
int aCount = sequence.count('A')
int tCount = sequence.count('T')

// Wallace rule calculation
double tm = 4 * (gCount + cCount) + 2 * (aCount + tCount)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
int gCount = sequence.count('G')
int cCount = sequence.count('C')
int aCount = sequence.count('A')
int tCount = sequence.count('T')
// Wallace rule calculation
double tm = 4 * (gCount + cCount) + 2 * (aCount + tCount)
int g_count = sequence.count('G')
int c_count = sequence.count('C')
int a_count = sequence.count('A')
int t_count = sequence.count('T')
// Wallace rule calculation
double tm = 4 * (g_count + c_count) + 2 * (a_count + t_count)

@robsyme does this work for you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants