Browse code

delete old files and pkg man page

Laurent Gatto authored on 17/04/2019 15:02:49
Showing1 changed files
1 1
deleted file mode 100644
... ...
@@ -1,170 +0,0 @@
1
-Hi,
2
-
3
-after beeing busy in the past weeks, we have now 
4
-overhauled most of the package. To me the most important 
5
-change is the possibility to have multiple backends, 
6
-such as netCDF, Ramp and pwiz to do the actual I/O work.
7
-
8
-We also have a patch for XCMS to do the I/O work 
9
-through mzR, and Laurent also has patches to msnbase 
10
-to switch to mzR. Both will be applied to BioC SVN
11
-if we have a few weeks of debugging before the release.
12
-
13
-Could you guys please add the Roundup user "lgatto" 
14
-to the nosy list of this issue ? Thanks in advance.
15
-
16
-Yours,
17
-Steffen
18
-
19
-On Mon, 2011-03-07 at 22:27 +0000, herve BioC-Submit wrote:
20
-herve <hpages@fhcrc.org> added the comment:
21
-...
22
-> Detailed review:
23
-> 
24
-> 1. Please use a BioC-style version number i.e. x.y.z
25
->    Note that if you use 0.99.z now, your package will be released as 1.0.0
26
->    in BioC 2.8 (to be released in April).
27
-Done.
28
- 
29
-> 2. The source tarball you provided is not clean: it contains a lot of object
30
->    files (i.e. *.o files) in src/boost and src/pwiz. A source
31
-I've added an mzR/cleanup file
32
-
33
-> 3. Please add a biocViews field to your DESCRIPTION file.
34
-Done. Taken from xcms and affyIO
35
-
36
-> 4. It doesn't make sense to have 2 SystemRequirements fields. Please merge
37
->    them.
38
-Done. Copy&Paste error :-(
39
- 
40
-> 5. Please put the Rcpp package in the Imports field of the DESCRIPTION
41
->    file. In your NAMESPACE file you import things from Rcpp so you
42
->    definitely want to reflect this in the Imports field. Keeping
43
-Done. 
44
-
45
->    in the Depends field is up to you but it could be that you don't need
46
->    this. (Do you want Rcpp to end up in the search path of the user when
47
->    s/he does library(mzR)? Are there symbols or man pages defined in Rcpp
48
->    that the end-user of the mzR package might need to access directly?)
49
-I have placed Rcpp in the dependency (v 0.5.0) and removed import(Rcpp)
50
-from the NAMESPACE file, to supress a warning about missing '"C++Object"
51
-unavailable and created in local env' when loading the package.
52
- 
53
-> 6. msdata needs to be put in the Suggests field: it's used in the vignette
54
->    and the unit tests.
55
-Done.
56
- 
57
-> 7. Same thing for RUnit and faahKO: they are used in the unit tests.
58
-Done.
59
- 
60
-> 8. Why call those files AllClass.R and rampMethods.R if the former
61
->  doesn't
62
-All of that was more/less rewritten by Laurent, 
63
-I guess it'll be cleaner now.
64
-
65
-> 9. In man/Ramp-class.Rd:
66
-> 
67
->      Objects can be created by calls of the form \code{new(Ramp)}
68
-> 
69
->    Will the user really create object like this? If yes, then please provide
70
->    an example on the \examples{} section of the man page.
71
-There is now a proper constructor (openMSfile, see below) that will read 
72
-an mass spec file, initialise the Rcpp module and return a proper S4 class.
73
- 
74
-> 10. The rampOpenFile() function itself (which is more likely to be *the* way
75
->     people will create Ramp objects) is not documented. In particular, the
76
->     documentation is not saying what the supported formats are (only
77
->     mzXML is used in the example and vignette).
78
-Done. rampOpenFile() is now called openMSfile() and documented 
79
-with its own manpage.
80
- 
81
-> 11. Overall the documentation is insufficient: there are only 2 small man
82
->     pages and none of them contains *real* examples. Note that man pages
83
->     should contain an \examples{} section, not an \section{Example}{} section
84
->     like you did in man/Ramp-class.Rd, so they are evaluated by 'R CMD check'.
85
->     The list of C++-style methods provided in man/Ramp-class.Rd gives very
86
->     little details about what those methods do exactly and it uses some
87
->     technical jargon that probably only experts in the field will understand:
88
-> 
89
->       "suppress RAMP's behavior of creating sparse tables to accomodate
90
->        unlisted scans"
91
-> 
92
->       "obtains minimal info on the msRun contained in the file"
93
-> 
94
->       "msLevel", "retentionTime"
95
-> 
96
->     If the man page cannot contain a full explanation of those terms, maybe
97
->     at least it should provide some reference to a document describing them.
98
-Done.
99
-
100
- 
101
-> 12. One more problem with man/Ramp-class.Rd: the get3DMap method doesn't
102
->     seem to exist:
103
-> 
104
->       > ramp$get3DMap(1)
105
->       Error in ramp$get3DMap(1) : could not find valid method
106
-> 
107
->     But maybe this is just a documentation problem: according to the man
108
->     page, it takes only 1 argument, when, in fact, 4 args seem to be required.
109
-all these methods that were called through the Rcpp module are now 
110
-encapsulated in S4 methods, all of which are documented.
111
-
112
-
113
-> 13. The vignette too is insufficient. It doesn't provide any context, and,
114
->     in particular, it doesn't provide any explanation about the special nature
115
->     of the classes that are used here (and the special syntax used to call
116
->     their methods). Providing some background information about the very
117
->     special nature of those classes/methods would be more than welcome.
118
-Done.
119
-
120
- 
121
-> 14. The .onLoad hook contains a setMethod() statement. This is the only
122
->     explicit use that I see of the S4 class system. That means mzR should
123
->     import the methods package i.e. have it in the Imports field, and have
124
->     import(methods) in the NAMESPACE file (generally right after the
125
->     useDynLib directive).
126
-Done.
127
- 
128
-> 15. I get the 2 following warnings when running 'R CMD check mzR_0.4.tar.gz':
129
-> 
130
->       * checking if this is a source package ... WARNING
131
->       Subdirectory ‘src’ contains:
132
->         README RcppRamp.hpp cramp.hpp
133
->       These are unlikely file names for src files.
134
-README went into inst/doc, and about *.hpp:
135
-
136
-Page 14 of http://cran.r-project.org/doc/manuals/R-exts.pdf 
137
-says "We recommend using ‘.h’ for headers, also for C++",
138
-with a note about potential not-guaranteed-portability.
139
-
140
-However, the *.hpp are valid C++ header files, 
141
-and it is also used throughout the boost libraries, 
142
-which is a high quality C++ project, some of which 
143
-will go into the next C++ standards. 
144
-
145
-So complaints about *.hpp is definitely a false positive.
146
-
147
-Nevertheless we have changed src/*hpp to src/*h for now.   
148
-
149
->     probably something you want to complain about on the R-devel mailing list,
150
-Will do.
151
-
152
->     and
153
-> 
154
->       * checking whether the name space can be loaded with stated dependencies
155
-> ... WARNING
156
->       Error: .onLoad failed in loadNamespace() for 'mzR', details:
157
->         call: MS$Ramp
158
->         error: could not find function "loadMethod"
159
->       Execution halted
160
-> 
161
->       A namespace must be able to be loaded with just the base namespace
162
->       loaded: otherwise if the namespace gets loaded by a saved object, the
163
->       session will be unable to start.
164
-> 
165
->       Probably some imports need to be declared in the NAMESPACE file.
166
-> 
167
->     probably something that will go away by importing the methods package (as
168
->     mentioned previously).
169
-Done. Rcpp stuff has been overhauled by Laurent.
170
-
Browse code

Added the mzR package to the repository.

git-svn-id: https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/mzR@57456 bc3139a8-67e5-0310-9ffc-ced21a209358

c.wong authored on 15/08/2011 19:53:29
Showing1 changed files
1 1
new file mode 100644
... ...
@@ -0,0 +1,170 @@
1
+Hi,
2
+
3
+after beeing busy in the past weeks, we have now 
4
+overhauled most of the package. To me the most important 
5
+change is the possibility to have multiple backends, 
6
+such as netCDF, Ramp and pwiz to do the actual I/O work.
7
+
8
+We also have a patch for XCMS to do the I/O work 
9
+through mzR, and Laurent also has patches to msnbase 
10
+to switch to mzR. Both will be applied to BioC SVN
11
+if we have a few weeks of debugging before the release.
12
+
13
+Could you guys please add the Roundup user "lgatto" 
14
+to the nosy list of this issue ? Thanks in advance.
15
+
16
+Yours,
17
+Steffen
18
+
19
+On Mon, 2011-03-07 at 22:27 +0000, herve BioC-Submit wrote:
20
+herve <hpages@fhcrc.org> added the comment:
21
+...
22
+> Detailed review:
23
+> 
24
+> 1. Please use a BioC-style version number i.e. x.y.z
25
+>    Note that if you use 0.99.z now, your package will be released as 1.0.0
26
+>    in BioC 2.8 (to be released in April).
27
+Done.
28
+ 
29
+> 2. The source tarball you provided is not clean: it contains a lot of object
30
+>    files (i.e. *.o files) in src/boost and src/pwiz. A source
31
+I've added an mzR/cleanup file
32
+
33
+> 3. Please add a biocViews field to your DESCRIPTION file.
34
+Done. Taken from xcms and affyIO
35
+
36
+> 4. It doesn't make sense to have 2 SystemRequirements fields. Please merge
37
+>    them.
38
+Done. Copy&Paste error :-(
39
+ 
40
+> 5. Please put the Rcpp package in the Imports field of the DESCRIPTION
41
+>    file. In your NAMESPACE file you import things from Rcpp so you
42
+>    definitely want to reflect this in the Imports field. Keeping
43
+Done. 
44
+
45
+>    in the Depends field is up to you but it could be that you don't need
46
+>    this. (Do you want Rcpp to end up in the search path of the user when
47
+>    s/he does library(mzR)? Are there symbols or man pages defined in Rcpp
48
+>    that the end-user of the mzR package might need to access directly?)
49
+I have placed Rcpp in the dependency (v 0.5.0) and removed import(Rcpp)
50
+from the NAMESPACE file, to supress a warning about missing '"C++Object"
51
+unavailable and created in local env' when loading the package.
52
+ 
53
+> 6. msdata needs to be put in the Suggests field: it's used in the vignette
54
+>    and the unit tests.
55
+Done.
56
+ 
57
+> 7. Same thing for RUnit and faahKO: they are used in the unit tests.
58
+Done.
59
+ 
60
+> 8. Why call those files AllClass.R and rampMethods.R if the former
61
+>  doesn't
62
+All of that was more/less rewritten by Laurent, 
63
+I guess it'll be cleaner now.
64
+
65
+> 9. In man/Ramp-class.Rd:
66
+> 
67
+>      Objects can be created by calls of the form \code{new(Ramp)}
68
+> 
69
+>    Will the user really create object like this? If yes, then please provide
70
+>    an example on the \examples{} section of the man page.
71
+There is now a proper constructor (openMSfile, see below) that will read 
72
+an mass spec file, initialise the Rcpp module and return a proper S4 class.
73
+ 
74
+> 10. The rampOpenFile() function itself (which is more likely to be *the* way
75
+>     people will create Ramp objects) is not documented. In particular, the
76
+>     documentation is not saying what the supported formats are (only
77
+>     mzXML is used in the example and vignette).
78
+Done. rampOpenFile() is now called openMSfile() and documented 
79
+with its own manpage.
80
+ 
81
+> 11. Overall the documentation is insufficient: there are only 2 small man
82
+>     pages and none of them contains *real* examples. Note that man pages
83
+>     should contain an \examples{} section, not an \section{Example}{} section
84
+>     like you did in man/Ramp-class.Rd, so they are evaluated by 'R CMD check'.
85
+>     The list of C++-style methods provided in man/Ramp-class.Rd gives very
86
+>     little details about what those methods do exactly and it uses some
87
+>     technical jargon that probably only experts in the field will understand:
88
+> 
89
+>       "suppress RAMP's behavior of creating sparse tables to accomodate
90
+>        unlisted scans"
91
+> 
92
+>       "obtains minimal info on the msRun contained in the file"
93
+> 
94
+>       "msLevel", "retentionTime"
95
+> 
96
+>     If the man page cannot contain a full explanation of those terms, maybe
97
+>     at least it should provide some reference to a document describing them.
98
+Done.
99
+
100
+ 
101
+> 12. One more problem with man/Ramp-class.Rd: the get3DMap method doesn't
102
+>     seem to exist:
103
+> 
104
+>       > ramp$get3DMap(1)
105
+>       Error in ramp$get3DMap(1) : could not find valid method
106
+> 
107
+>     But maybe this is just a documentation problem: according to the man
108
+>     page, it takes only 1 argument, when, in fact, 4 args seem to be required.
109
+all these methods that were called through the Rcpp module are now 
110
+encapsulated in S4 methods, all of which are documented.
111
+
112
+
113
+> 13. The vignette too is insufficient. It doesn't provide any context, and,
114
+>     in particular, it doesn't provide any explanation about the special nature
115
+>     of the classes that are used here (and the special syntax used to call
116
+>     their methods). Providing some background information about the very
117
+>     special nature of those classes/methods would be more than welcome.
118
+Done.
119
+
120
+ 
121
+> 14. The .onLoad hook contains a setMethod() statement. This is the only
122
+>     explicit use that I see of the S4 class system. That means mzR should
123
+>     import the methods package i.e. have it in the Imports field, and have
124
+>     import(methods) in the NAMESPACE file (generally right after the
125
+>     useDynLib directive).
126
+Done.
127
+ 
128
+> 15. I get the 2 following warnings when running 'R CMD check mzR_0.4.tar.gz':
129
+> 
130
+>       * checking if this is a source package ... WARNING
131
+>       Subdirectory ‘src’ contains:
132
+>         README RcppRamp.hpp cramp.hpp
133
+>       These are unlikely file names for src files.
134
+README went into inst/doc, and about *.hpp:
135
+
136
+Page 14 of http://cran.r-project.org/doc/manuals/R-exts.pdf 
137
+says "We recommend using ‘.h’ for headers, also for C++",
138
+with a note about potential not-guaranteed-portability.
139
+
140
+However, the *.hpp are valid C++ header files, 
141
+and it is also used throughout the boost libraries, 
142
+which is a high quality C++ project, some of which 
143
+will go into the next C++ standards. 
144
+
145
+So complaints about *.hpp is definitely a false positive.
146
+
147
+Nevertheless we have changed src/*hpp to src/*h for now.   
148
+
149
+>     probably something you want to complain about on the R-devel mailing list,
150
+Will do.
151
+
152
+>     and
153
+> 
154
+>       * checking whether the name space can be loaded with stated dependencies
155
+> ... WARNING
156
+>       Error: .onLoad failed in loadNamespace() for 'mzR', details:
157
+>         call: MS$Ramp
158
+>         error: could not find function "loadMethod"
159
+>       Execution halted
160
+> 
161
+>       A namespace must be able to be loaded with just the base namespace
162
+>       loaded: otherwise if the namespace gets loaded by a saved object, the
163
+>       session will be unable to start.
164
+> 
165
+>       Probably some imports need to be declared in the NAMESPACE file.
166
+> 
167
+>     probably something that will go away by importing the methods package (as
168
+>     mentioned previously).
169
+Done. Rcpp stuff has been overhauled by Laurent.
170
+