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

Fix type comparisons #1382

Merged
merged 2 commits into from
Aug 7, 2023
Merged

Fix type comparisons #1382

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75f862b
fix type comparisons
bernt-matthias Aug 4, 2023
41065f6
check for dict
bernt-matthias Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion planemo/autoupdate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ def check_tool_step(step, ts): # return a dict with current and newest tool ver
return {}

outdated_tool_dict = {}
steps = wf_dict["steps"].values() if type(wf_dict["steps"]) == dict else wf_dict["steps"]
steps = wf_dict["steps"].values() if isinstance(wf_dict["steps"], dict) else wf_dict["steps"]
for step in steps:
if step.get("type", "tool") == "tool" and not step.get("run", {}).get("class") == "GalaxyWorkflow":
outdated_tool_dict.update(check_tool_step(step, ts))
Expand Down
2 changes: 1 addition & 1 deletion planemo/galaxy/workflows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ def rewrite_job_file(input_file, output_file, job):
with open(input_file) as f:
job_contents = yaml.safe_load(f)
for job_input, job_input_name in job_contents.items():
if type(job[job_input]) == dict: # dataset or collection
if isinstance(job[job_input], dict): # dataset or collection
job_contents[job_input] = {"class": job_input_name["class"], "galaxy_id": job[job_input]["id"]}
# else: presumably a parameter, no need to modify
with open(output_file, "w") as f:
Expand Down
6 changes: 3 additions & 3 deletions planemo/workflow_lint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,12 @@ def _lint_best_practices(path: str, lint_context: WorkflowLintContext) -> None:
"""

def check_json_for_untyped_params(j):
values = j if type(j) == list else j.values()
values = j if isinstance(j, list) else j.values()
Copy link
Member

Choose a reason for hiding this comment

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

Can you check this is a dict instance instead ? that should be less narrow. Also the check in line 196 isn't really how you'd do this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you check this is a dict instance instead ?

So is TYPE everywhere?

Also the check in line 196 isn't really how you'd do this

Also with is?

Copy link
Member

Choose a reason for hiding this comment

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

No, type is type is the most narrow of them all which will not work for any subclasses. .values() will only work for objects that satisfy Mapping, while any j that is Iterable will work. So check for the specific instance where you need to call .values() first isinstance(j, Mapping) and don't worry about the other types that will all be fine.

bernt-matthias marked this conversation as resolved.
Show resolved Hide resolved
for value in values:
if type(value) in [list, dict, OrderedDict]:
if check_json_for_untyped_params(value):
return True
elif type(value) == str:
elif isinstance(value, str):
if re.match(r"\$\{.+?\}", value):
return True
return False
Expand Down Expand Up @@ -345,7 +345,7 @@ def _tst_input_valid(
input_def: Dict[str, Any],
lint_context: WorkflowLintContext,
) -> bool:
if type(input_def) == dict: # else assume it is a parameter
if isinstance(input_def, dict): # else assume it is a parameter
clazz = input_def.get("class")
if clazz == "File":
input_path = input_def.get("path")
Expand Down
Loading