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

Detect & accept array-only JSON files from legacy MapKnitter #1350

Closed
jywarren opened this issue Feb 8, 2023 · 41 comments · Fixed by #1358
Closed

Detect & accept array-only JSON files from legacy MapKnitter #1350

jywarren opened this issue Feb 8, 2023 · 41 comments · Fixed by #1358

Comments

@jywarren
Copy link
Member

jywarren commented Feb 8, 2023

I just realized also - the cached JSON files from legacy MapKnitter use the plain [...] array format 😭

https://web.archive.org/web/20220726175911/https://mapknitter.org/maps/ceres--2/warpables.json

This doesn't have to be a problem for us, certainly not now. The format we've chosen is definitely better:

{
  collection: [
    { src: "url", nodes: [...] },
    { src: "url", nodes: [...] },
    { src: "url", nodes: [...] }
  ]
}

We can add a feature which can detect if the JSON is just an array, and accept that format too, for backwards compatibility. We can work on it later!

@jywarren
Copy link
Member Author

jywarren commented Feb 8, 2023

If we develop this, then we can enable people to open old MapKnitter maps, by adding /warpables.json to the end of any of these maps whose URLs we saved:

https://gist.github.com/jywarren/8cc1ee8f68a46e227eeb86ca068fdf0c

In the format:

https://web.archive.org/web/0id_/https://mapknitter.org/maps/_________/warpables.json

example for raw response: https://web.archive.org/web/20220726175911id_/https://mapknitter.org/maps/ceres--2/warpables.json#

That'll be pretty cool.

@jywarren
Copy link
Member Author

jywarren commented Feb 8, 2023

With about 7000 maps there, we could even allow a mini search engine -- maybe something like https://twitter.github.io/typeahead.js/examples/, but we don't have to think about that yet.

@segun-codes
Copy link
Collaborator

Sounds interesting, I will be happy to work on this once PR #1345 is given final landing.

@segun-codes
Copy link
Collaborator

Hi @jywarren, I will now begin work to convert this to PR. Many thanks.

@jywarren
Copy link
Member Author

Once we get this working, we can really enjoy looking at a lot of old maps, complex ones, and we can use that to write up a nice post showing off all your new work!

@jywarren
Copy link
Member Author

I see that this code results in a CORS error!

axios.get("https://web.archive.org/web/0id_/https://mapknitter.org/maps/ceres--2/warpables.json")
  .then((response) => {
    console.log(response.data);
    console.log(response.status);
    console.log(response.statusText);
    console.log(response.headers);
    console.log(response.config);
  });

Hmm. I think we need another API endpoint...

@jywarren
Copy link
Member Author

We could just fetch every map with https://web.archive.org/web/0id_/<URL> format requests, and save all the JSON files on a Public Lab server, with CORS permissions. Or even in a GH repository?

@segun-codes
Copy link
Collaborator

I see that this code results in a CORS error!

axios.get("https://web.archive.org/web/0id_/https://mapknitter.org/maps/ceres--2/warpables.json")
  .then((response) => {
    console.log(response.data);
    console.log(response.status);
    console.log(response.statusText);
    console.log(response.headers);
    console.log(response.config);
  });

Hmm. I think we need another API endpoint...

Yeah, I ran into CORS errors while trying to fix PR #1358 (as this task is part of this PR as I planned it). IA does not seem to provide such endpoint, I combed almost everywhere but haven't found any such endpoint.

@segun-codes
Copy link
Collaborator

We could just fetch every map with https://web.archive.org/web/0id_/<URL> format requests, and save all the JSON files on a Public Lab server, with CORS permissions. Or even in a GH repository?

I guess those are good candidate solutions. I was planning to tell you about the CORS errors, the error led me into focusing on DnD first.

@jywarren
Copy link
Member Author

Great, yes, this kind of makes sense on the CORS stuff. But we can think on it a bit - yes, i agree this is a great first step forward!

@segun-codes
Copy link
Collaborator

We could just fetch every map with https://web.archive.org/web/0id_/<URL> format requests, and save all the JSON files on a Public Lab server, with CORS permissions. Or even in a GH repository?

On a second thought, I'm wondering how this will work. Does web.archive.org support programmatic download of the images? It works if the images are saved in a location of this URL format archive.org/download/**/***.png .

@jywarren
Copy link
Member Author

It's true, I'm not sure. Right now I believe all the images are actually in an Amazon S3 bucket -- see the URLS like:

https://s3.amazonaws.com/grassrootsmapping/warpables/306184/DJI_1204_medium.JPG

(From https://web.archive.org/web/20220726175911id_/https://mapknitter.org/maps/ceres--2/warpables.json)

So they will load fine. But the images are also all present in the archive.org, like:

https://web.archive.org/web/0id_/https://s3.amazonaws.com/grassrootsmapping/warpables/306184/DJI_1204_medium.JPG

It looks like those are also CORS limited. I tweeted about this to see if anyone knows, and I have a contact at archive.org I can ask if we don't hear anything: https://twitter.com/jywarren/status/1625984206869348352

@segun-codes
Copy link
Collaborator

Okay... I'd wait for any feedback from IA to enable next move.

@jywarren
Copy link
Member Author

Well, if we don't hear soon, we can also create a copy of all the JSON files at a CORS-enabled location, for the short term.

@segun-codes
Copy link
Collaborator

Absolutely... many possible alternatives. No problem!

@jywarren
Copy link
Member Author

Reopening for follow-ups!

@jywarren jywarren reopened this Feb 15, 2023
@jywarren
Copy link
Member Author

So, I tried this - with this JSON file:

https://web.archive.org/web/20220726175911id_/https://mapknitter.org/maps/ceres--2/warpables.json

Which should look like this: https://web.archive.org/web/20220726175945/https://mapknitter.org/maps/ceres--2/edit

But it didn't quite work... it's close!

image

Here's what it's supposed to look like:

image

Actually only 3 images are placed, and the remainder we don't have to display on the map at all. I thought I saw there was a check for whether corners exist, so I was surprised to see that more images were placed. This is so exciting though! @segun-codes what do you think might be happening?

@segun-codes
Copy link
Collaborator

segun-codes commented Feb 16, 2023

Oh! interesting so:

  1. Do you mean we should ignore images without corners and that they should not be placed on the tile? My implementation is such that images without corners are placed on the center of the map using auto-generated coordinates, hence the behaviour you observed.
  2. As for the weird looking image, I will have to investigate, ordinarily map.fitBound() should have addressed this if it was incorrectly rendered. In the earlier PR, I dropped a gif showing this same thing you complained about. I thought the image was made that way deliberately by a user of MK service.

@jywarren
Copy link
Member Author

Yes that's right, in old map knitter maps the JSON file recorded images that were uploaded but not yet used.
Maybe removing those will fix the remaining image?

@segun-codes
Copy link
Collaborator

Ok, so great! I'd give this a shot. By the way, any feedback from twitter or IA team? Many thanks!

@jywarren
Copy link
Member Author

jywarren commented Feb 16, 2023 via email

@jywarren
Copy link
Member Author

I'm also now generating a list of all the JSON files from all MapKnitter legacy maps, with this URL schema:

https://archive.publiclab.org/mk-json/--10.json

We can also filter through those and prefix all the image URLs with https://web.archive.org/web/0id_ too if we want, making it completely Archive based for images as well!

@segun-codes
Copy link
Collaborator

Okay, does it mean you are manually saving all the json files to archive.org now?

@jywarren
Copy link
Member Author

I'm saving them to a public lab server right now but we can try to figure out where to put them next! Yes, I guess we could put them into an archive.org collection... Good idea!

@segun-codes
Copy link
Collaborator

segun-codes commented Feb 16, 2023

I filtered out image objects without coordinates but the issue with the weird looking image persists. Can't see the effect of map.fitBounds() this time. Let me open a PR for this.

@jywarren
Copy link
Member Author

jywarren commented Feb 16, 2023 via email

@segun-codes
Copy link
Collaborator

segun-codes commented Feb 17, 2023

Yeah, I did that yesterday. Basically, all the images in ceres--2 behaved this way. I went a step further to isolate one of the image objects into a stand alone .json file then dragged and dropped on the tile. So again it was the weird look, the coordinates came out fine. See:

Illustration 1:
Illustration 1

@segun-codes
Copy link
Collaborator

segun-codes commented Feb 17, 2023

I noticed the longitude values are about 144degrees. Then, the JSON files we have experimented with successfully in the past have low order magnitude of longitude (e.g., see sample corner set below). Now, given the view area restriction here map = L.map('map').setView([51.505, -0.09], 13), are sections of the image on higher order magnitude of longitude not supposed to be affected/cut-off/improperly rendered? It seems so to me, not entire sure though.

Sample Corner Set
{"lat": 51.51376371940495, "lon": -0.20565032958984375},{"lat": 51.51376371940495,"lon": -0.12805938720703128},{"lat": 51.47828253510034,"lon": -0.20565032958984375},{"lat": 51.47828253510034,"lon": -0.12805938720703128}

Then I observed, the longitude data is not written to json file when button 'save' is clicked. I'd do some more checks now.

@segun-codes
Copy link
Collaborator

Just to mention, generateJson() can now handle longitude written as lng or lon.

@jywarren
Copy link
Member Author

jywarren commented Feb 17, 2023 via email

@segun-codes
Copy link
Collaborator

segun-codes commented Feb 18, 2023

I'm thinking the longitude values must undergo some form of transformation into lower order values like -0.2xxx before we pass it to map.fitBound(). @jywarren, have you any thought? I don't have any new ideas right now.

@jywarren
Copy link
Member Author

Hmm, strange. I'm trying to explicitly set these, but it's still not changing the distorted display:

function placeImage (imageURL, options, newImage = false) {
  let image;
  
  if (newImage) { // Construct new map
    image = L.distortableImageOverlay(
      imageURL,
      {tooltipText: options.tooltipText}
    );
  } else if (options.corners) { // Reconstruct map to previous saved state
    image = L.distortableImageOverlay(
      imageURL,
      {
        tooltipText: options.tooltipText,
        corners: [
          L.latLng(options.corners[0], options.corners[0]),
          L.latLng(options.corners[1], options.corners[1]),
          L.latLng(options.corners[2], options.corners[2]),
          L.latLng(options.corners[3], options.corners[3])
        ]
      }
    );
    console.log('corners',image.getCorners())
  }
  if (image) map.imgGroup.addLayer(image);
  
};
$ map._layers[184].getCorners() 
=>
[
    {
        "lat": -37.7664063648,
        "lng": 144.9828654528
    },
    {
        "lat": -37.7650239004,
        "lng": 144.9831980467
    },
    {
        "lat": -37.7652020107,
        "lng": 144.9844533205
    },
    {
        "lat": -37.7665844718,
        "lng": 144.9841207266
    }
]

@jywarren
Copy link
Member Author

I also tried manually running:

i = L.distortableImageOverlay(
      "https://s3.amazonaws.com/grassrootsmapping/warpables/306187/DJI_1207.JPG",
      {
        corners: [
    {
        "lat": -37.7664063648,
        "lng": 144.9828654528
    },
    {
        "lat": -37.7650239004,
        "lng": 144.9831980467
    },
    {
        "lat": -37.7652020107,
        "lng": 144.9844533205
    },
    {
        "lat": -37.7665844718,
        "lng": 144.9841207266
    }
]}).addTo(map);

And saw the same thing... so strange!!

@jywarren
Copy link
Member Author

Strange, so I tried rotating the order of the corner points, and it changes the direction the error happens:

image

@jywarren
Copy link
Member Author

aha -- we have an order of corners issue!

image

I tried doing a different order and got this!!!

@jywarren
Copy link
Member Author

OK, so it's still not quite right. But the corner order is important. We should compare to the

    image = L.distortableImageOverlay(
      imageURL,
      {
        tooltipText: options.tooltipText,
        corners: [
          L.latLng(options.corners[1], options.corners[1]),
          L.latLng(options.corners[2], options.corners[2]),
          L.latLng(options.corners[0], options.corners[0]),
          L.latLng(options.corners[3], options.corners[3])
        ]
      }
    );
    console.log('corners',image.getCorners())

MapKnitter had accounted for this, with order 0,1,3,2:

https://github.com/publiclab/mapknitter/blob/3a1e332721c59754fb3ef7638f8169e71953791e/app/assets/javascripts/mapknitter/Map.js#L87C26-L92

Yes, that's correct!!

    image = L.distortableImageOverlay(
      imageURL,
      {
        tooltipText: options.tooltipText,
        corners: [
          L.latLng(options.corners[0], options.corners[0]),
          L.latLng(options.corners[1], options.corners[1]),
          L.latLng(options.corners[3], options.corners[3]),
          L.latLng(options.corners[2], options.corners[2])
        ]
      }
    );

OK, so actually we need to standardize everything to that. We should follow the previous convention, including our own JSON exporting. That will mean we won't have to have 2 different logics to do this.

Phew, that was a tough one!!!

@segun-codes
Copy link
Collaborator

segun-codes commented Feb 20, 2023

Waoh! very good observation, well done @jywarren. All along, I had thought the order of coordinates in MK were consistent with the order in MK Lite.

@segun-codes
Copy link
Collaborator

segun-codes commented Feb 20, 2023

Okay, so I see the coordinates must be passed in this order NW, NE, SW, SE order (in a "Z" shape).

  1. I'd try to review generateExportJson() to ensure it aligns with this convention.
  2. Then, I'd also try to retrieve Json files from https://archive.org/download/mapknitter/...

Let's see how all these will pan out.

@jywarren
Copy link
Member Author

Great. I'll just note that https://archive.org/download/mapknitter is now all online and present, all 7824 maps.

@jywarren
Copy link
Member Author

This is now working, and we could think about the best way to display a list of all the maps in https://archive.org/download/mapknitter/, but they all work now! Great work!!!!

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