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

Test clipBoth does not raise exception for invalid filenames #170

Closed
uniomni opened this issue Jun 4, 2012 · 5 comments
Closed

Test clipBoth does not raise exception for invalid filenames #170

uniomni opened this issue Jun 4, 2012 · 5 comments
Assignees

Comments

@uniomni
Copy link
Contributor

uniomni commented Jun 4, 2012

Came across this when moving data files from inasafe_data/test to inasafe_data/hazard

The test clipBoth does not raise an exception when the filename is missing - rather it gives the misleading message
"Western boundary must be less than eastern. I got [0.0, 0.0, 0.0, 0.0]"

The issue is with the code

myRasterLayer = QgsRasterLayer(RASTERPATH, myName)
msg = 'Did not find layer "%s" in path "%s"' % (myName, RASTERPATH)
assert myRasterLayer is not None, msg

It turns out that QgsRasterLayer (or QgsVectorLayer for that matter) do not return None when filename is invalid.
I made a specific test for this (disabled for the time being):

test_invalid_filenames_caught(self):

So think there are 3 options:

  • make the qgis calls return None
  • make the qgis calls raise an exception
  • work out how to query the result to identify this situation.
@ghost ghost assigned timlinux Jun 4, 2012
@timlinux
Copy link
Contributor

timlinux commented Jun 4, 2012

Hi

Oh! Ok we can use layer.isValid() to check they are ok. There is no return type since in the underlying C++ implementation , constructors always have a void return type.

Regards

Tim

@uniomni
Copy link
Contributor Author

uniomni commented Jun 4, 2012

Yep great - I thought it'd be something like that....
So the QGIS functions always return layer instances right?
Then one has to check for validity rather than None.

Cheers
Ole

On Mon, Jun 4, 2012 at 5:36 PM, Tim Sutton <
reply@reply.github.com

wrote:

Hi

Oh! Ok we can use layer.isValid() to check they are ok. There is no return
type since in the underlying C++ implementation , constructors always have
a void return type.

Regards

Tim


Reply to this email directly or view it on GitHub:
#170 (comment)

@uniomni
Copy link
Contributor Author

uniomni commented Jun 5, 2012

I have changed the test to use .isValid() in 76f59d1
However, I noticed that the constructors for Vector and Raster data both print error statements that mix with with the test output. Also the statements are different for the two layer types:

For Raster layers the error message is

ERROR 4: `OEk_tnshoeu_439_kstnhoe' does not exist in the file system,
and is not recognised as a supported dataset name.

QgsRasterLayer::setDataProvider: Data provider is invalid.

for vector layers it is simply

Data source is invalid

In case of non-existing files, I think both Python bindings should either

  • be using exceptions,
  • return None or
  • be silent and rely on .isValid()

uniomni added a commit that referenced this issue Jun 5, 2012
@uniomni
Copy link
Contributor Author

uniomni commented Jun 10, 2012

Seems we both did the same changes (see above). How does that work?

@uniomni
Copy link
Contributor Author

uniomni commented Jun 10, 2012

Attempting to suppress qgis textual errors was behind commit #34a583c6d4b72abc4479a9a8416e1d5be3c379eb but seems to have caused problem in issue #176

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

No branches or pull requests

2 participants