LabraMabra / M2019_4135

Data Processing and Visualization with R
3 stars 2 forks source link

Maria_Firuleva_HWs #8

Open faramer86 opened 5 years ago

faramer86 commented 5 years ago

HW_02

Good job, but:

1) dplyr and tidyverse are pretty cool, but are not really necessary here. (* As far as I understand it was written in the task that it is forbidden to use those)

2) Your function is slightly "over-argumented":) colNames or colIndexes are pretty much the same. It is not necessary to create new variable name in local space just because it has different data type. If you want, you can specify data types. But nevertheless.. this allows you to combine different data types as indexes (For example, use character and numeric column indexing at the same time, cool, but I hope you will never use it!). It is cool that you came up with such perverted example)

3) Also, try this one: dataSubset(mtcars,rowIndexes=1,colNames="gear"). Where is the data?

All in all, nice, but I have to consult Lera.

faramer86 commented 5 years ago

Sorry, Lera said that she told you not to use external packages and use only base functions (without *pply). Unfortunetely, you have to redo this task.

mariafiruleva commented 5 years ago

@faramer86 , thanks for your detailed review!

  1. It wasn't mentioned in the original task ( I re-writed my code.

  2. I totally agree with you! However, the task postulates:

Subsets data according to the given selections. Possible selections are:

  • By index/indexes
  • By name/names
  • By logical vector

Because there is no information about combinations of possible selections, I've tried to realize all possible combinations which can make sense, with some restrictions described in the function description.

  1. As mentioned in function description,
  • colIndexes: vector of column indexes which you want to save in result dataframe after filtering using colNames; make sure there is no conflict between these two steps; by default, it includes all column indexes in input dataframe;

I guess, the most correct way to catch these type of errors is appending informative logs. Now if you try to call the function with irrelevant order of arguments, you will see an error message with an explained source of error.

As a result, I've changed HW01.Rmd file (remove ply, add logs, etc). Also, I add splitting, optional calculations, etc in file HW02.Rmd*.

faramer86 commented 5 years ago

You've done a great job, but I really think that you are continuing complicate things)

1) I really don't get how I can select only one column?))

dataSubset(iris, colIndexes = 1, rowIndexes = c(1:12))

*If I screwed up with arguments, just tell me, where I'm wrong)

P.S. Could you please rename HW01 -> HW02 and HW02 -> HW03. Since they are task2 and task3) It will really help me!

faramer86 commented 5 years ago

HW_03

1) If you fix last issue in HW_02 - write here.

2) I'm looking forward to get your HW_03.

mariafiruleva commented 5 years ago

@faramer86 , sorry for the late reply, I didn't notice it :(

  1. Fixed. Thanks!
  2. I uploaded HW_03 a week ago (commit e0a8e5c) as HW_04 file (because you asked to rename the previous files)

Also, I rename all previous files (HW01 -> HW02, HW02 -> HW03).

faramer86 commented 4 years ago

HW_03

Let it be OK.

HW_04 (weather df tidying)

Great! But.. Is it a good idea to store time data like so? Google "lubridate" package? Fix it and I'm OK with HW_04.

faramer86 commented 4 years ago

HW_04

Good.

HW_05

I'm OK with your solution.

HW_06

Part A:

Issues:

1) Your realization of iris_wide -> iris_long is not optimal. By this moment it is expected that you know basics of dplyr. Based on your previous HWs you know this library. Try to use the following logic:

"Good" style:

iris %>%
  gather() %>%
  separate() %>%
  mutate() %>%
  spread()

"Bad" style:

long_iris <- cbind(gather(select(iris, matches(stringr::regex("Length|Species"))), Part_L, Length, -Species),
      gather(select(iris, matches(stringr::regex("Width"))), Part_W, Width)) %>% separate(Part_L, "Part") %>% select(-Part_W)

Part B:

1) First plot. X axis - definitely not like in reference. Hint: scale_x.... 2) Second plot. Y axis - same as 1 issue. Hint: ylim. 2) Third plot. Y axis - same as 1 issue. Hint: ylim.

HW_07

Issues:

1) Second plot. Y axis - compare it with reference. Hint: free.

HW_APP

Issues:

1) I have this displayed all the time: argument is of length zero. 2) Also summary and subsetted data isn't shown.

mariafiruleva commented 4 years ago

@faramer86

HW_06:

Part A: fixed Also, I decided to use pivot_longer instead of gather, because

Development on gather() is complete, and for new code we recommend switching to pivot_longer(), which is easier to use, more featureful, and still under active development. df %>% gather("key", "value", x, y, z) is equivalent to df %>% pivot_longer(c(x, y, z), names_to = "key", values_to = "value")

Part B: fixed

HW_07: fixed

HW_APP: For me, everything is okay. Perhaps you doesn't have some libraries, e.g., do you have shinyWidgets which I use in UI part?

faramer86 commented 4 years ago

HW_06

Part A: Yes, good choice. Part B: You still have some tiny differences with the references, but I'm OK with your solution)

HW_07

Cool.

HW_APP

Yeah, good of you. I don't have shinyWidgets. My fault. As for the app - Excellent.

P.S.

Great. All is done, congrats! Wish you good luck with your project.