search

zoonproject / zoon

The zoon R package
Other
61 stars 13 forks source link

New version of raster #423

Closed AugustT closed 5 years ago

AugustT commented 5 years ago

The new version of Raster causes an error in testing.

TL;DR - I need to tweak one of the tests

Conversation with raster maintainer:

Dear Tom,

The recent update of the R package "raster" causes a test error (due to an error not happening!) in your package "zoon". This happens in the "workflow" function, which is a bit trickier than other functions to understand and debug. See:

https://cran.r-project.org/web/checks/check_results_zoon.html

Could you have a look at let me know if this is due an error in "raster"?

Thanks, Robert

Hi Robert,

I think I have traced the change in behaviour of raster that is causing this.

In this scenario I have a raster I want to extract data from using points. I have subset my points in a way that returns a dataframe with 0 rows (assume I did something stupid). I then run raster::extract. Repex attached. I believe that this used to return NULL but now returns ‘numeric(0)’. This is causing zoon problems because it uses ‘is.null’ to catch this case and report an error and prevent the modelling progressing further.

Can you please confirm this change in behaviour of raster? Do you intend to keep it the way it is now? If so I will make changes to zoon’s catch to accommodate this change.

Tom

Hi Tom, Thanks for your quick reply. I found what causes this. In version 2.8-1 I have made a change so that it returns NULL again. However, may I suggest that you change your test to if (length(x) == 0) As that would work for both cases (NULL and and an empty vector)? Thanks, Robert

timcdlucas commented 5 years ago

Great work dealing with this so quickly.

Tim

On Mon, 22 Oct 2018, 11:04 Dr Tom August, notifications@github.com wrote:

The new version of Raster causes an error in testing.

TL;DR - I need to tweak one of the tests

Conversation with raster maintainer:

Dear Tom,

The recent update of the R package "raster" causes a test error (due to an error not happening!) in your package "zoon". This happens in the "workflow" function, which is a bit trickier than other functions to understand and debug. See:

https://cran.r-project.org/web/checks/check_results_zoon.html

Could you have a look at let me know if this is due an error in "raster"?

Thanks, Robert

Hi Robert,

I think I have traced the change in behaviour of raster that is causing this.

In this scenario I have a raster I want to extract data from using points. I have subset my points in a way that returns a dataframe with 0 rows (assume I did something stupid). I then run raster::extract. Repex attached. I believe that this used to return NULL but now returns ‘numeric(0)’. This is causing zoon problems because it uses ‘is.null’ to catch this case and report an error and prevent the modelling progressing further.

Can you please confirm this change in behaviour of raster? Do you intend to keep it the way it is now? If so I will make changes to zoon’s catch to accommodate this change.

Tom

Hi Tom, Thanks for your quick reply. I found what causes this. In version 2.8-1 I have made a change so that it returns NULL again. However, may I suggest that you change your test to if (length(x) == 0) As that would work for both cases (NULL and and an empty vector)? Thanks, Robert

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zoonproject/zoon/issues/423, or mute the thread https://github.com/notifications/unsubscribe-auth/ADavNHcukrkRDGiqUDdu0jNG0Q8YR2Ufks5unZhKgaJpZM4XzEkD .

AugustT commented 5 years ago

I havent actually implemented the fix yet!

From: Tim Lucas notifications@github.com Sent: 22 October 2018 18:20:27 To: zoonproject/zoon Cc: August, Tom; Assign Subject: Re: [zoonproject/zoon] New version of raster (#423)

Great work dealing with this so quickly.

Tim

On Mon, 22 Oct 2018, 11:04 Dr Tom August, notifications@github.com wrote:

The new version of Raster causes an error in testing.

TL;DR - I need to tweak one of the tests

Conversation with raster maintainer:

Dear Tom,

The recent update of the R package "raster" causes a test error (due to an error not happening!) in your package "zoon". This happens in the "workflow" function, which is a bit trickier than other functions to understand and debug. See:

https://cran.r-project.org/web/checks/check_results_zoon.html

Could you have a look at let me know if this is due an error in "raster"?

Thanks, Robert

Hi Robert,

I think I have traced the change in behaviour of raster that is causing this.

In this scenario I have a raster I want to extract data from using points. I have subset my points in a way that returns a dataframe with 0 rows (assume I did something stupid). I then run raster::extract. Repex attached. I believe that this used to return NULL but now returns ‘numeric(0)’. This is causing zoon problems because it uses ‘is.null’ to catch this case and report an error and prevent the modelling progressing further.

Can you please confirm this change in behaviour of raster? Do you intend to keep it the way it is now? If so I will make changes to zoon’s catch to accommodate this change.

Tom

Hi Tom, Thanks for your quick reply. I found what causes this. In version 2.8-1 I have made a change so that it returns NULL again. However, may I suggest that you change your test to if (length(x) == 0) As that would work for both cases (NULL and and an empty vector)? Thanks, Robert

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zoonproject/zoon/issues/423, or mute the thread https://github.com/notifications/unsubscribe-auth/ADavNHcukrkRDGiqUDdu0jNG0Q8YR2Ufks5unZhKgaJpZM4XzEkD .

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/zoonproject/zoon/issues/423#issuecomment-431903547, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ADzYbG5arx-qfdZHrYT3FrmsVWINDpO1ks5unf5bgaJpZM4XzEkD.

This message (and any attachments) is for the recipient only. NERC is subject to the Freedom of Information Act 2000 and the contents of this email and any reply you make may be disclosed by NERC unless it is exempt from release under the Act. Any material supplied to NERC may be stored in an electronic records management system.

timcdlucas commented 5 years ago

Oh ok! Well great work... handling it thus far?

On Tue, 23 Oct 2018, 16:08 Dr Tom August, notifications@github.com wrote:

I havent actually implemented the fix yet!

From: Tim Lucas notifications@github.com Sent: 22 October 2018 18:20:27 To: zoonproject/zoon Cc: August, Tom; Assign Subject: Re: [zoonproject/zoon] New version of raster (#423)

Great work dealing with this so quickly.

Tim

On Mon, 22 Oct 2018, 11:04 Dr Tom August, notifications@github.com wrote:

The new version of Raster causes an error in testing.

TL;DR - I need to tweak one of the tests

Conversation with raster maintainer:

Dear Tom,

The recent update of the R package "raster" causes a test error (due to an error not happening!) in your package "zoon". This happens in the "workflow" function, which is a bit trickier than other functions to understand and debug. See:

https://cran.r-project.org/web/checks/check_results_zoon.html

Could you have a look at let me know if this is due an error in "raster"?

Thanks, Robert

Hi Robert,

I think I have traced the change in behaviour of raster that is causing this.

In this scenario I have a raster I want to extract data from using points. I have subset my points in a way that returns a dataframe with 0 rows (assume I did something stupid). I then run raster::extract. Repex attached. I believe that this used to return NULL but now returns ‘numeric(0)’. This is causing zoon problems because it uses ‘is.null’ to catch this case and report an error and prevent the modelling progressing further.

Can you please confirm this change in behaviour of raster? Do you intend to keep it the way it is now? If so I will make changes to zoon’s catch to accommodate this change.

Tom

Hi Tom, Thanks for your quick reply. I found what causes this. In version 2.8-1 I have made a change so that it returns NULL again. However, may I suggest that you change your test to if (length(x) == 0) As that would work for both cases (NULL and and an empty vector)? Thanks, Robert

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zoonproject/zoon/issues/423, or mute the thread < https://github.com/notifications/unsubscribe-auth/ADavNHcukrkRDGiqUDdu0jNG0Q8YR2Ufks5unZhKgaJpZM4XzEkD

.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub< https://github.com/zoonproject/zoon/issues/423#issuecomment-431903547>, or mute the thread< https://github.com/notifications/unsubscribe-auth/ADzYbG5arx-qfdZHrYT3FrmsVWINDpO1ks5unf5bgaJpZM4XzEkD

.

This message (and any attachments) is for the recipient only. NERC is subject to the Freedom of Information Act 2000 and the contents of this email and any reply you make may be disclosed by NERC unless it is exempt from release under the Act. Any material supplied to NERC may be stored in an electronic records management system.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/zoonproject/zoon/issues/423#issuecomment-432283613, or mute the thread https://github.com/notifications/unsubscribe-auth/ADavNHgVxnay_gYtYT0yj0xMdKUZGzlSks5unzDigaJpZM4XzEkD .

timcdlucas commented 5 years ago

Seems like tests are passing so I assume this got fixed.