Skip to content
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

Fix errors and implement possible improvements arising from my suggestions for the paper #129

Open
2 of 8 tasks
eugenividal opened this issue Jan 12, 2025 · 5 comments
Open
2 of 8 tasks
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@eugenividal
Copy link
Collaborator

eugenividal commented Jan 12, 2025

  • In the README:

  • Rephrase the paragraph explaining the different versions of the data, as it is in the suggestions for the paper

  • Given that the title is done with one #, add one level (i.e. ##) to the headings 1.

  • plot the desire lines using the flow mapper.

  • Remove the title of the first figure --- there is already a caption with title for this figure.

  • Beautify Figure 4, checking for typos (e.g. munucipality, houlry), putting the first three boxes at the same level, etc.

  • In the vignettes:

  • the name of the mobility datasets could be clearer, specially '2.2. Number of trips data'. Rename them as follows:

     + Version 1, Mobility data:  
          - Origin-Destination data
          - Population by trip count data 
    
     +  Version 2, Mobility data:
           - Origin-Destination data
           - Population by trip count data
           - Population by overnight stay data
    
  • In the package:

  • Fix inconsistencies in the name of the variables of the different versions: e.g. date and full_date, and residence_province_name and residence_province.

  • I'd consider renaming some variables: E.g. time slot = hour_slot and distance = interval_distance

@eugenividal eugenividal changed the title Errors and possible improvements arising from my suggestions for the paper Fix errors and possible improvements arising from my suggestions for the paper Jan 12, 2025
@eugenividal eugenividal changed the title Fix errors and possible improvements arising from my suggestions for the paper Fix errors and implement possible improvements arising from my suggestions for the paper Jan 12, 2025
@Robinlovelace
Copy link
Collaborator

Great set of suggestions Eugeni, here are my thoughts:

  • Rephrase the paragraph explaining the different versions of the data: sounds good! The README can be concise so no need to include all the detail but agree that rephrasing the paragraph is a good idea.
  • Given that the title is done with one #, add one level (i.e. ##) to the headings 1.

👍

  • Not sure if @Robinlovelace will be happy with this, but for consistency, perhaps it may be better to plot the desire lines also using the flow mapper.

I vote keeping the use of the {od} package to show a wider range of use cases. Geographic desire lines can be useful

  • Remove the title of the first figure --- there is already a caption with title for this figure.

👍

And 👍 to all the others!

@Robinlovelace
Copy link
Collaborator

And to the offer of updates 👍

@e-kotov
Copy link
Member

e-kotov commented Jan 13, 2025

  1. @eugenividal sure, go on with the PR

  • Not sure if @Robinlovelace will be happy with this, but for consistency, perhaps it may be better to plot the desire lines also using the flow mapper.

I vote keeping the use of the {od} package to show a wider range of use cases. Geographic desire lines can be useful

@Robinlovelace I think @eugenividal only suggested to plot the desire lines with {flowmapper}, that does not mean removing {od}. Or are you worried that plotting with {flowmapper} would 'hide' most of the desire lines with low trip counts?

@Robinlovelace
Copy link
Collaborator

No objections to plotting the desire lines with {flowmapper}. Would make the figures in the README more consistent.

@eugenividal
Copy link
Collaborator Author

eugenividal commented Jan 13, 2025

only suggested to plot the desire lines with {flowmapper}

Exactly!

I'll carry on with this PR another day. I got distracted with another study.

@e-kotov e-kotov assigned e-kotov and eugenividal and unassigned e-kotov Jan 14, 2025
@e-kotov e-kotov added the documentation Improvements or additions to documentation label Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants