Skip to content

Automatically convert integers to doubles when argument type is double#823

Merged
rcannood merged 6 commits into
develop_0_8from
nxf_fix_integer_as_double
Apr 24, 2025
Merged

Automatically convert integers to doubles when argument type is double#823
rcannood merged 6 commits into
develop_0_8from
nxf_fix_integer_as_double

Conversation

@rcannood
Copy link
Copy Markdown
Contributor

@rcannood rcannood commented Apr 23, 2025

Describe your changes

This PR allows users to pass an integer to a double argument without Nextflow throwing an error about it being the wrong type.

This fix needs to be ported to Viash 0.9 and dev as well.

Related issue(s)

Closes #xxxx

Type of Change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New functionality (non-breaking change which adds functionality)
  • Major change (non-breaking change which modifies existing functionality)
  • Minor change (non-breaking change which does not modify existing functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

Checklist

Requirements:

  • I have read the CONTRIBUTING doc.
  • I have performed a self-review of my code by checking the "Changed Files" tab.
  • My code follows the code style of this project.

Tests:

  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

Documentation:

  • Proposed changes are described in the CHANGELOG.md.
  • I have updated the documentation accordingly.

Test Environment

@rcannood rcannood force-pushed the nxf_fix_integer_as_double branch from 5946aa3 to dece396 Compare April 23, 2025 06:48
@rcannood rcannood requested a review from DriesSchaumont April 23, 2025 09:00
Comment thread CHANGELOG.md Outdated
value = value.doubleValue()
}
if (value instanceof Float) {
if (value instanceof Float || value instanceof Integer || value instanceof Long) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is Short and/or Double also a problem?

Copy link
Copy Markdown
Contributor Author

@rcannood rcannood Apr 23, 2025

Choose a reason for hiding this comment

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

I tried to resolve this by using a more idiomatic casting mechanism, namely:

  // ...
  } else if (par.type == "double") {
    // cast to double if need be
    if (value !instanceof Double) {
      try {
        value = value as Double
      } catch (NumberFormatException e) {
        expectedClass = "Double"
      }
    }
  } // ...

WDYT?

rcannood and others added 2 commits April 23, 2025 11:43
Co-authored-by: Dries Schaumont <5946712+DriesSchaumont@users.noreply.github.com>
@rcannood rcannood requested a review from DriesSchaumont April 23, 2025 09:59
Copy link
Copy Markdown
Contributor

@DriesSchaumont DriesSchaumont left a comment

Choose a reason for hiding this comment

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

Looks OK. One thing I am wondering about is how hard it would be to refactor the if else structure into a hashmap and a loop.

@rcannood
Copy link
Copy Markdown
Contributor Author

let's do additional improvements in viash develop, or even in nf-viash ^^

@rcannood rcannood merged commit aad8e66 into develop_0_8 Apr 24, 2025
12 of 13 checks passed
rcannood added a commit that referenced this pull request Apr 24, 2025
rcannood added a commit that referenced this pull request Apr 24, 2025
* post release changes, update release guide

* port of #823

* add pr number

* copied 0.8.8 changelog entry

* update version and changelog

* Update CHANGELOG.md

* Update CHANGELOG.md

---------

Co-authored-by: Robrecht Cannoodt <rcannood@gmail.com>
colobas pushed a commit to colobas/viash that referenced this pull request May 13, 2025
rcannood pushed a commit that referenced this pull request Jun 12, 2025
#832)

* Fix author names in paper (#644)

* add paper

* update paper

* add more orcids

* move figures

* rename pipeline to workflow

* add section on benefits

* rewrite state of the field

* solve issue with figure 3

* update bibtex

* update bibtex again

* broaden domain

* update figure 2 based on @wilkinson's feedback

* minor changes in paper (#628)

* minor change

* fix refactoring error

* Add JOSS release info

* update readme

* Fix author names

---------

Co-authored-by: Daniel S. Katz <d.katz@ieee.org>

* use reflection to get viash to fix issue in nextflow edge (#812)

* use reflection to get viash to fix issue in nextflow edge

* add entry to changelog

* Backport: Make sure scripts in Nextflow have the right extension (#815)

* Backport: Make sure scripts in Nextflow have the right extension

* fix changelog

* Edit -- I'm going to port it in the different direction

* Pass session as an argument (#818)

* pass session as an argument

* fix import

* Prepare for release

* post release changes, update release guide

* Add entry for 0.8.7

* port of #823

* add pr number

* copied 0.8.8 changelog entry

* update version and changelog

* Update CHANGELOG.md

* Update CHANGELOG.md

* post release changes

* cherry pick 5271a3b

* fix test

* Remove deprecated things (#833)

* wip deprecate things

* fix components

* fix test

* update tests and yamls

* fix test (recent versions of git require a message)

* remove test code for removed functionality

* Update CHANGELOG.md

---------

Co-authored-by: Hendrik Cannoodt <hendrik.cannoodt@gmail.com>
Co-authored-by: Daniel S. Katz <d.katz@ieee.org>
@rcannood rcannood deleted the nxf_fix_integer_as_double branch October 2, 2025 19:55
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.

2 participants