Skip to content

Conversation

@germa89
Copy link
Collaborator

@germa89 germa89 commented Oct 20, 2025

Description

Fix nodelist parsing issue. Setting default to None in get_value.

Issue linked

Close #4266
Close #4232

Checklist

@germa89 germa89 requested a review from a team as a code owner October 20, 2025 09:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a nodelist parsing issue in the SLURM integration by making type conversion in get_slurm_options more flexible. The changes address issue #4266 by modifying the get_value helper function to preserve default value types and handle string values appropriately.

Key Changes:

  • Made astype parameter optional (defaulting to None instead of int)
  • Added logic to infer type from default parameter when astype is not specified
  • Explicitly passed astype=str for SLURM_NODELIST to ensure string handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov
Copy link

codecov bot commented Oct 20, 2025

Codecov Report

❌ Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (709e72b) to head (f925c8f).
⚠️ Report is 2 commits behind head on main.

❌ Your patch status has failed because the patch coverage (33.33%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4267      +/-   ##
==========================================
- Coverage   91.32%   91.30%   -0.03%     
==========================================
  Files         193      193              
  Lines       15720    15725       +5     
==========================================
+ Hits        14357    14358       +1     
- Misses       1363     1367       +4     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added the bug Issue, problem or error in PyMAPDL label Oct 20, 2025
@germa89
Copy link
Collaborator Author

germa89 commented Oct 22, 2025

@pyansys-ci-bot LGTM

@germa89 germa89 enabled auto-merge (squash) October 22, 2025 09:27
Copy link
Contributor

@pyansys-ci-bot pyansys-ci-bot left a comment

Choose a reason for hiding this comment

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

✅ Approving this PR because germa89 said so in here 😬

LGTM

@germa89 germa89 disabled auto-merge October 22, 2025 09:28
@germa89 germa89 linked an issue Oct 22, 2025 that may be closed by this pull request
4 tasks
@germa89 germa89 enabled auto-merge (squash) October 22, 2025 11:25
@germa89 germa89 merged commit a35c784 into main Oct 22, 2025
86 of 88 checks passed
@germa89 germa89 deleted the fix/slurm-nodelist-parsing-issue branch October 22, 2025 11:52
clatapie pushed a commit that referenced this pull request Oct 27, 2025
…lt type. (#4267)

* fix(slurm): make get_slurm_options astype optional and preserve default type.

* chore: adding changelog file 4267.fixed.md [dependabot-skip]

* fix(slurm): allow non-integer SLURM option values by trying float then falling back to str

---------

Co-authored-by: pyansys-ci-bot <[email protected]>
clatapie pushed a commit that referenced this pull request Nov 5, 2025
…lt type. (#4267)

* fix(slurm): make get_slurm_options astype optional and preserve default type.

* chore: adding changelog file 4267.fixed.md [dependabot-skip]

* fix(slurm): allow non-integer SLURM option values by trying float then falling back to str

---------

Co-authored-by: pyansys-ci-bot <[email protected]>
clatapie pushed a commit that referenced this pull request Nov 18, 2025
…lt type. (#4267)

* fix(slurm): make get_slurm_options astype optional and preserve default type.

* chore: adding changelog file 4267.fixed.md [dependabot-skip]

* fix(slurm): allow non-integer SLURM option values by trying float then falling back to str

---------

Co-authored-by: pyansys-ci-bot <[email protected]>
clatapie pushed a commit that referenced this pull request Nov 21, 2025
…lt type. (#4267)

* fix(slurm): make get_slurm_options astype optional and preserve default type.

* chore: adding changelog file 4267.fixed.md [dependabot-skip]

* fix(slurm): allow non-integer SLURM option values by trying float then falling back to str

---------

Co-authored-by: pyansys-ci-bot <[email protected]>
clatapie pushed a commit that referenced this pull request Nov 21, 2025
…lt type. (#4267)

* fix(slurm): make get_slurm_options astype optional and preserve default type.

* chore: adding changelog file 4267.fixed.md [dependabot-skip]

* fix(slurm): allow non-integer SLURM option values by trying float then falling back to str

---------

Co-authored-by: pyansys-ci-bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue, problem or error in PyMAPDL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ValueError when using SLURM_NODELIST launch_mapdl throws ValueError in HPC, attempts to cast SLURM_NODELIST to int

3 participants