Trackage / trip

trip package development
http://trackage.github.io/trip/
12 stars 2 forks source link

pre-retirement noise from retiring packages #52

Open rsbivand opened 1 year ago

rsbivand commented 1 year ago

Running revdeps on https://github.com/rsbivand/sp/tree/sp161, https://r-forge.r-project.org/R/?group_id=884 and similar trunk for rgeos and maptools, and after moving the package startup messages for retiring packages to onLoad(), I see: 00check.log with: testthat.Rout.zip provoked I think by sp pulling in rgdal:

> library(trip)
The legacy packages maptools, rgdal, and rgeos, underpinning this package
will retire shortly. Please refer to R-spatial evolution reports on
https://r-spatial.org/r/2023/05/15/evolution4.html for details.
This package is now running under evolution status 0 
>   tms <- (Sys.time() + 1:100)[-(40:60)]
> tr <- walrus818[1:600, ]
> filt1 <- speedfilter(tr, max.speed = 2000)
Please note that rgdal will be retired during October 2023,
plan transition to sf/stars/terra functions using GDAL and PROJ
at your earliest convenience.
See https://r-spatial.org/r/2023/05/15/evolution4.html and https://github.com/r-spatial/evolution
rgdal: version: 1.6-7, (SVN revision 1202)
Geospatial Data Abstraction Library extensions to R successfully loaded
Loaded GDAL runtime: GDAL 3.7.0, released 2023/05/02
Path to GDAL shared files: /usr/local/share/gdal
 GDAL does not use iconv for recoding strings.
GDAL binary built with GEOS: TRUE 
Loaded PROJ runtime: Rel. 9.2.0, March 1st, 2023, [PJ_VERSION: 920]
Path to PROJ shared files: /home/rsb/.local/share/proj:/usr/local/share/proj:/usr/local/share/proj
PROJ CDN enabled: FALSE
Linking to sp version:1.6-1
To mute warnings of possible GDAL/OSR exportToProj4() degradation,
use options("rgdal_show_exportToProj4_warnings"="none") before loading sp or rgdal.

Probably expect_silent could be dropped, at least for now?

mdsumner commented 1 year ago

thanks Roger, super diligent

just retire bro 🙏😇 I learnt a lot, appreciated

rsbivand commented 1 year ago

Next step is sp 1.6-1 soon (to increase the noise level), then noisy rgeos, rgdal and maptools, early June sp 2.0.0 using sf not rgdal by default (evolution status 2L) but still letting users set 0L to test against rgdal. Then archive rgeos, rgdal and maptools in October. Some more hints for perople in https://r-spatial.org/r/2023/05/15/evolution4.html, but I'm still worried about legacy workflows with inattentive authors, see: https://twitter.com/BobOHara/status/1649032540173238272, which @Nowosad sent me. Lots can be made safe even with sp objects using coercion to and from sf or terra, but that may be scary for those simply running scripts others write for them. Any ideas how to seed SEO to point queries like "R spatial broken" to the evolution blogs?

mdsumner commented 1 year ago

it's a weird message

image

"this package" has no context when loaded by a reverse dep - and sp underpins the retiring packages, not the way it's worded now

rsbivand commented 1 year ago

Thanks, I should have seen that. Will change to the sp from this for 2.0-0 due in ten days. Maybe adding loaded directly or indirectly too. 2.1-0 in October dropping retiring packages will keep a message, but should I add an option and or environment variable to turn it off? I think a message is needed in workflows that may fail, but am very grateful for input about how or whether to do this.

mdsumner commented 1 year ago

I don't think there should be such a message on load, on attach yes

rsbivand commented 1 year ago

Thanks! I tried to adopt these helpful suggestions in: https://github.com/rsbivand/sp/tree/sp200, https://github.com/rsbivand/sp/commit/bd980dfe371ee47b626b6f0a3ea6702ac4f226d3, over and above flipping the default evolution status to 2L, due to be submitted 10 June. With these improvements, I see:

$ _SP_STARTUP_MESSAGE_="attach" R
...
> loadNamespace("sp")
<environment: namespace:sp>
> library(sp)
The legacy packages maptools, rgdal, and rgeos, underpinning the sp package,
which was just attached, will retire in October 2023.
Please refer to R-spatial evolution reports for details, especially
https://r-spatial.org/r/2023/05/15/evolution4.html.
The sp package is now running under evolution status 2
     (status 2 uses the sf package in place of rgdal)

and

$ _SP_STARTUP_MESSAGE_="none" R
...
> loadNamespace("sp")
<environment: namespace:sp>
> library(sp)

The three values are "load" (default), "none" or "attach". They can also be given by options("sp_startup_message"="load"), etc. before sp is loaded. Does that give enough flexibility?

rsbivand commented 1 year ago

Following up https://github.com/r-spatial/evolution/issues/10 and @bastistician 's careful CMD check with _R_CHECK_SUGGESTS_ONLY_=true, I'm afraid that because sp::is.projected in evolution status 2L, soon to be default, and if sf isn't suggested in trip, three tests get caught: 00check.log. With these edits: trip_sp2.zip, (adding sf to Suggests: and conditioning the tests on sf being on the library path), CMD check passes whether sf is available or not. This based on trip_1.9.0.tar.gz published yesterday. CMD check also works if _SP_STARTUP_MESSAGE_="none" is set as an environment variable before sp is loaded. This with reference to https://github.com/edzer/sp/pull/135. Very sorry for the mess, grateful to @bastistician for trying _R_CHECK_SUGGESTS_ONLY_=true before we submit sp 2.0-0.

mdsumner commented 1 year ago

BDR told me a {graticule} issue only showed with that suggests var but I couldn't trigger it or understand what he meant. Perhaps he mixed up graticule and trip and saw this.

I'll check, all good 🙏

rsbivand commented 1 year ago

I'll check graticule tomorrow my time.

rsbivand commented 1 year ago

All OK for graticule_0.3.0.tar.gz with current CRAN sp 1.6-1 and my fork at 2.0-0, both with and without sf on the library path. Maybe yes, it was trip not graticule.

mdsumner commented 1 year ago

ok, me neither - I can't trigger errors with that env var for either

is there anything here blocking you? I'm happy to roll with whatever CRAN needs as it comes up

rsbivand commented 1 year ago

Nothing blocking here, I agree that we can take it as it goes. @bastistician 's intervention about _R_CHECK_SUGGESTS_ONLY_=true turned up almost a dozen packages that might break if re-submitted to CRAN for sp 2.0-0; I'm contacting them, but most were already warned about the retirement process.