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.

Sign up for GitHub

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

Jump to bottom

Special treatment empty POSIXPath parameters #731

Merged
merged 14 commits into from
Jan 28, 2025
Merged

Special treatment empty POSIXPath parameters #731

merged 14 commits into from
Jan 28, 2025

Conversation

burnout87
Copy link
Collaborator

No description provided.

@burnout87 burnout87 linked an issue Jan 20, 2025 that may be closed by this pull request
True optianal PosixPath parameter #730
Closed
@burnout87
Copy link
Collaborator Author

Along with oda-hub/nb2workflow#229

@burnout87
Copy link
Collaborator Author

burnout87 commented Jan 20, 2025 •
edited
Loading

With this PR, the arg force_default_value for the Parameter class is introduced: if this is set, then we the default value is forced and applied, but only in the case such parameter is not part of the input form.

This is, for now, used only for the POSIXPath class, where we want to avoid the situation described in esg-epfl-apc/galaxy#4 (comment)

@codecov Codecov
Copy link

codecov bot commented Jan 20, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.31%. Comparing base (3ccd4f4) to head (233b294).
Report is 15 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
+ Coverage   62.18%   62.31%   +0.12%     
==========================================
  Files          50       51       +1     
  Lines        9151     9170      +19     
==========================================
+ Hits         5691     5714      +23     
+ Misses       3460     3456       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@burnout87
Copy link
Collaborator Author

burnout87 commented Jan 20, 2025 •
edited
Loading

The goal, is to obtain this - img_file_path set to an empty string

Screenshot from 2025-01-20 15-50-49

@burnout87
Copy link
Collaborator Author

Following oda-hub/nb2workflow#229 with this PR, we set a default value for the POSIXPath parameters in the case this is not included in the request.

This is, because we want to avoid the situation described in esg-epfl-apc/galaxy#4 (comment)

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 23, 2025
View reviewed changes
cdci_data_analysis/plugins/dummy_plugin/exposer.py
@@ -4,6 +4,7 @@
from . import empty_development_instrument
from . import empty_async_return_progress_instrument
from . import empty_instrument_with_conf
from . import empty_instrument_with_posix_path

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'empty_instrument_with_posix_path' is not used.
tests/test_server_basic.py
}

p_file_path = DispatcherJobState.create_p_value_file(p_value=6)
list_file = open(p_file_path)

Check warning

Code scanning / CodeQL

File is not always closed Warning test

File may not be closed if an exception is raised.
@burnout87 burnout87 mentioned this pull request Jan 27, 2025
Merged
@burnout87 burnout87 requested a review from dsavchenko January 27, 2025 10:00
dsavchenko
dsavchenko approved these changes Jan 27, 2025
View reviewed changes
Copy link
Member

@dsavchenko dsavchenko left a comment

Choose a reason for hiding this comment

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

The title of the PR may be updated to better reflect what are changes finally

@burnout87 burnout87 changed the title force overwrite of the default value for POSIXPath paramters Special treatment empty POSIXPath parameters Jan 27, 2025
@burnout87 burnout87 merged commit 230dc27 into master Jan 28, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@volodymyrss volodymyrss volodymyrss approved these changes

@dsavchenko dsavchenko dsavchenko approved these changes

Assignees
No one assigned
Labels
test-oda-api test-plugins
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

True optianal PosixPath parameter
3 participants
@burnout87 @volodymyrss @dsavchenko