-
Notifications
You must be signed in to change notification settings - Fork 4
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
11 automate spatial feature specification #14
11 automate spatial feature specification #14
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this refactoring! It is much easier to follow the logic for each method option even if some method may just be two lines of code.
Please see some comments and suggestions.
R/preprocessing.R
Outdated
} | ||
spFeatures <- spFeatures[,featureNames] | ||
|
||
featureNames <- intersect(featureNames,colnames(spFeatures)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There could be a case where user lists 6 patterns but we find only 5 in the object. How should we handle that?
- Message which reports that we did not find all patterns, and are continuing with the rest.
- Error which reports the specific patterns we did not find, and ask user to check input.
I note that this scenario may be more common in non-pipeline use cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed in cd2ab20
refactor:
featureNames=NULL
since checking with regex"."
does the same and is shorterif else
and messagesready to automate spatial feature specification now.