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

Fix shape val #163

Merged
merged 8 commits into from
Nov 25, 2021
Merged

Fix shape val #163

merged 8 commits into from
Nov 25, 2021

Conversation

constantinpape
Copy link
Contributor

Fixes the input/output shape validation which did not work at all; we just did not notice because the tests for test_model where also broken...
In addition this also fixes the return codes from all CLI functions, which always returned 0 before.

@github-actions github-actions bot added the bug Something isn't working label Nov 25, 2021
constantinpape and others added 2 commits November 25, 2021 11:56
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@constantinpape
Copy link
Contributor Author

Switching to the correct return codes has unearthed another bug: test_cli_package[unet2d_nuclei_broad_model] fails with

Traceback (most recent call last):                                                                                                                                                                                                                     
  File "/home/pape/Work/bioimageio/spec-bioimage-io/bioimageio/spec/shared/fields.py", line 425, in _deserialize                                                                                                                                       
    return raw_nodes.URI(uri_string=value)                                                                                                                                                                                                             
  File "<string>", line 9, in __init__                                                                                                                                                                                                                 
  File "/home/pape/Work/bioimageio/spec-bioimage-io/bioimageio/spec/shared/raw_nodes.py", line 109, in __post_init__                                                                                                                                   
    raise ValueError("Invalid URI or relative path. (use URI with scheme 'file' for absolute file paths)")                                                                                                                                             
ValueError: Invalid URI or relative path. (use URI with scheme 'file' for absolute file paths)                                                                                                                                                         
                                                                                                                                                                                                                                                       
The above exception was the direct cause of the following exception:                                                                                                                                                                                   
                                                                                                                                                                                                                                                       
Traceback (most recent call last):                                                                                                                                                                                                                     
  File "/home/pape/Work/bioimageio/python-bioimage-io/bioimageio/core/commands.py", line 36, in package                                                                                                                                                
    rdf_local_source = resolve_uri(rdf_source)                                                                                                                                                                                                         
  File "/home/pape/Work/software/conda/miniconda3/envs/bio-core-dev/lib/python3.7/functools.py", line 840, in wrapper                                                                                                                                  
    return dispatch(args[0].__class__)(*args, **kw)                                                                                                                                                                                                    
  File "/home/pape/Work/bioimageio/python-bioimage-io/bioimageio/core/resource_io/utils.py", line 161, in _resolve_uri_str                                                                                                                             
    return resolve_uri(fields.URI().deserialize(uri), root_path)                                                                                                                                                                                       
  File "/home/pape/Work/software/conda/miniconda3/envs/bio-core-dev/lib/python3.7/site-packages/marshmallow/fields.py", line 364, in deserialize                                                                                                       
    output = self._deserialize(value, attr, data, **kwargs)                                                                                                                                                                                            
  File "/home/pape/Work/bioimageio/spec-bioimage-io/bioimageio/spec/shared/fields.py", line 427, in _deserialize                                                                                                                                       
    raise ValidationError(value) from e                                                                                                                                                                                                                
marshmallow.exceptions.ValidationError: /tmp/bioimageio_cache/packages/UNet_2D_Nuclei_Broad_0_1_3p37.zip                                                                                                                                               
                                                                                                                                                                                                                                                       
During handling of the above exception, another exception occurred:          

This looks like an old problem that I hoped was fixed ....
@FynnBe I have no idea about this part, so I would suggest we merge this despite the test error (given that the CLI tests were just wrong beforehand) so that you can work on fixing this in a different PR (I am also not sure if this needs to be fixed here or in spec).

@FynnBe
Copy link
Member

FynnBe commented Nov 25, 2021

This error stems from our mixed use of the URI classes (node and field) as paths and specifically that we allow relative paths, but no absolute ones. That we ended up with that has historic reasons only (e.g. we tried to avoid the marshmallow_union package)
To really clean this up we need to be strict about the use of URIs (a uri needs a scheme, thus any path is not a valid URI!) and use Union[URI, Path] where appropriate and enforce that paths are relative where they should be. (currently the URI just always raises on absolute paths which leads to these problems)

@constantinpape
Copy link
Contributor Author

Is this fixd by bioimage-io/spec-bioimage-io#287 or do we need to change something else here?

@constantinpape
Copy link
Contributor Author

Is this fixd by bioimage-io/spec-bioimage-io#287 or do we need to change something else here?

No, looks like it still fails.

@FynnBe
Copy link
Member

FynnBe commented Nov 25, 2021

Is this fixd by bioimage-io/spec-bioimage-io#287 or do we need to change something else here?

no, but hopefully by this WIP bioimage-io/spec-bioimage-io#288 and its future equivalent here for bioimageio.core

@FynnBe FynnBe merged commit 8b99cee into main Nov 25, 2021
@FynnBe FynnBe deleted the fix-shape-val branch November 25, 2021 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants