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

"Integration of modularized export code" #1

Merged
merged 15 commits into from
Apr 12, 2019
Merged

"Integration of modularized export code" #1

merged 15 commits into from
Apr 12, 2019

Conversation

jywarren
Copy link
Member

@jywarren jywarren commented Mar 29, 2019

This will fail for now 😱 but it starts to use the new Gem! https://github.com/publiclab/mapknitter-exporter

@jywarren
Copy link
Member Author

This'll need a good bit of work re: publiclab/mapknitter-exporter#5 !

@jywarren
Copy link
Member Author

Images will need structure like this:

https://github.com/publiclab/mapknitter-exporter/blob/bf375b6f2cb09070503f523d24ba803936144875/test/exporter_test.rb#L15-L39

A lot of this can come from the JSON

@jywarren
Copy link
Member Author

@jywarren
Copy link
Member Author

jywarren commented Mar 29, 2019

Currently JSON provides:

{
"cm_per_pixel":4.99408,
"created_at":"2019-02-22T05:56:05Z",
"deleted":false,
"height":2976,
"history":"",
"id":306187,
"image_content_type":"image/jpeg",
"image_file_name":"DJI_1207.JPG",
"image_file_size":8148349,
"locked":false,
"map_id":13015,
"nodes":[
  {"author":"anonymous","body":null,"color":"black","created_at":"2019-02-27T23:13:20Z","description":"","id":2593754,"lat":"-37.7664063648","lon":"144.9828654528","map_id":0,"name":"","order":0,"updated_at":"2019-02-27T23:13:20Z","way_id":0,"way_order":0},
  {"author":"anonymous","body":null,"color":"black","created_at":"2019-02-27T23:13:20Z","description":"","id":2593755,"lat":"-37.7650239004","lon":"144.9831980467","map_id":0,"name":"","order":0,"updated_at":"2019-02-27T23:13:20Z","way_id":0,"way_order":0},
  {"author":"anonymous","body":null,"color":"black","created_at":"2019-02-27T23:13:20Z","description":"","id":2593756,"lat":"-37.7652020107","lon":"144.9844533205","map_id":0,"name":"","order":0,"updated_at":"2019-02-27T23:13:20Z","way_id":0,"way_order":0},
  {"author":"anonymous","body":null,"color":"black","created_at":"2019-02-27T23:13:20Z","description":"","id":2593757,"lat":"-37.7665844718","lon":"144.9841207266","map_id":0,"name":"","order":0,"updated_at":"2019-02-27T23:13:20Z","way_id":0,"way_order":0}
],
"parent_id":null,
"thumbnail":null,
"updated_at":"2019-02-27T23:13:21Z",
"width":3968,
"src":"https://s3.amazonaws.com/grassrootsmapping/warpables/306187/DJI_1207.JPG",
"srcmedium":"https://s3.amazonaws.com/grassrootsmapping/warpables/306187/DJI_1207_medium.JPG"
}

cm_per_pixel would be provided, but could (as fallback) come from https://github.com/publiclab/mapknitter/blob/628b8b56f12dab83ee2b2c0439556eaa3b8b4d17/app/models/warpable.rb#L94-L109

But for now, I believe the only fields we'll actually need will be (the rest are not relevant, or could be calculated from the images we fetch):

{
"cm_per_pixel":4.99408,
"id":306187, // some unique id
// "image_file_size":8148349, // we could use this later maybe, to estimate run time...
"nodes":[ 
  {"id":2593754,"lat":"-37.7664063648","lon":"144.9828654528"},
  {"id":2593755,"lat":"-37.7650239004","lon":"144.9831980467"},
  {"id":2593756,"lat":"-37.7652020107","lon":"144.9844533205"},
  {"id":2593757,"lat":"-37.7665844718","lon":"144.9841207266"}
],
"src":"https://s3.amazonaws.com/grassrootsmapping/warpables/306187/DJI_1207.JPG",
}

@jywarren
Copy link
Member Author

jywarren commented Mar 29, 2019

OK, so... because we need these for the whole collection, not just for each image, we need to expand warpables.json format that MapKnitter provides, to include:

      @data['user_id'],
      @data['resolution'],
      export,
      @data['id'],

Right now, warpables.json is just an array. We need it to be a JSON object, so like:

{
  'user_id': 1, // this is not needed for the export but on reading it back we can re-associate it with a user
  'resolution': 11.11, // optional resolution for whole export -- we can also just average all the images
  'id': 1, // this is not needed for the export process but on reading it back we can re-associate it with a map/export record
  'images': [
    {
    "cm_per_pixel": 4.99408,
    "id":306187, // some unique id relevant to the Rails app; optional and we could default to timestamp
    // "image_file_size": 8148349, // we could use this later maybe, to estimate run time...
    "nodes": [ 
      {"id":2593754,"lat":"-37.7664063648","lon":"144.9828654528"}, // id is also optional here
      {"id":2593755,"lat":"-37.7650239004","lon":"144.9831980467"},
      {"id":2593756,"lat":"-37.7652020107","lon":"144.9844533205"},
      {"id":2593757,"lat":"-37.7665844718","lon":"144.9841207266"}
    ],
    "src":"https://s3.amazonaws.com/grassrootsmapping/warpables/306187/DJI_1207.JPG",
    },
    { another image },
    { another image }
  ]
}

So, if we use timestamps or random numbers for the id fields, absolute minimal could be (and is already provided by https://mapknitter.org/maps/ceres--2/warpables.json):

{
  'images': [
    {
    "cm_per_pixel": 4.99408,
    "nodes": [ 
      {"lat":"-37.7664063648","lon":"144.9828654528"}, // id is also optional here
      {"lat":"-37.7650239004","lon":"144.9831980467"},
      {"lat":"-37.7652020107","lon":"144.9844533205"},
      {"lat":"-37.7665844718","lon":"144.9841207266"}
    ],
    "src":"https://s3.amazonaws.com/grassrootsmapping/warpables/306187/DJI_1207.JPG",
    }
  ]
}

Let's call this export.json to distinguish from status.json

@icarito
Copy link
Member

icarito commented Apr 10, 2019

Okay I believe I've finally linked mapknitter-exporter from the git repository from the Gemfile. I can import the library and invoke run_export!!! 🚀 🎉 🎈 🏄‍♂️

But as you mentioned, this has the wrong number of arguments because of publiclab/mapknitter-exporter#6 - and also we need to settle the warpable.json format...

@jywarren your guidance much appreciated in these parts - tests are running too!

@data[0]['height'],
export,
@data[0]['map_id'],
@data[0]['images'], # TODO: these images need a special format like https://github.com/publiclab/mapknitter-exporter/blob/bf375b6f2cb09070503f523d24ba803936144875/test/exporter_test.rb#L15-L39
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we resolve this comment and rely on their formatting coming properly from the incoming JSON request?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! We need an instance of a properly format JSON request.


attr_accessor :status, :tms, :geotiff, :zip, :jpg # these will be updated with i.e. export.tms = "/path"

def save
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here, we should be updating a .json file stored in google cloud - but can we merge now, and add this on in a later PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please.

# }

# simplified this because of https://github.com/publiclab/mapknitteexporter/pull/6... it won't work yet though
MapKnitterExporter.run_export(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is the one whose parameters will change. But can we peg to the current version of mapknitter-exporter for now, and then when we change the params, we'll issue a breaking change version bump and then make the change here too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I am linking to the current (git) version of mapknitter-exporter except to the add_cartagen branch because otherwise it wouldn't require. Please merge the add_cartagen branch if it is correct.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, cool, could you open a PR for this? Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jywarren
Copy link
Member Author

This is awesome! Wow, is there a place we can try this out with a JSON file?

@icarito
Copy link
Member

icarito commented Apr 11, 2019

This is awesome! Wow, is there a place we can try this out with a JSON file?

Yes, I just pushed / deployed to the google cloud using "Google Run" :
https://mapknitter-exporter-d6yfsgjd2q-uc.a.run.app/

It seems to work! ** (edit: to the point it needs to work until now...)

@icarito
Copy link
Member

icarito commented Apr 11, 2019

So let's merge this then if you agree!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants