hablapps / doric

Type safety for spark columns
https://www.hablapps.com/doric/
Apache License 2.0
76 stars 11 forks source link

[Bug report]: some API website links are not working #235

Closed eruizalo closed 1 year ago

eruizalo commented 2 years ago

Som API pages does not work, returning a 404 error: ~https://www.habla.dev/doric/docs/api/3.2/scala-2.12/api/doric/BinaryOperationsSyntax32.html~ updated: https://www.habla.dev/doric/docs/api/spark-3.3/scala-2.12/doric/BinaryOperationsSyntax32.html

We should take a look to the whole webpage and see why this happens and how to fix it: ~https://www.habla.dev/doric/docs/api/3.2/scala-2.12/api/doric/index.html~ updated: https://www.habla.dev/doric/docs/api/spark-3.3/scala-2.12/doric/index.html

Amalicia commented 1 year ago

Hey folks! 👋

It's been a little while!

Two things I've noticed which are kind of related to this issue:

  1. The Scala doc links in general don't seem to be working anymore - perhaps this should be an issue on it's own but thought better to reach out first just in case this was expected.
  2. I've been playing around a little bit locally and it seems setting a scope of package skips the Scala doc generation. This is just a theory right now and I need to dig into this a little bit more but thought I should share my findings just in case someone else starts looking into this
alfonsorr commented 1 year ago

Yup, you are right, we've made a change in the path of the doc to make it easier to maintain and those links don't work anymore, this would be the valid one https://www.habla.dev/doric/docs/api/spark-3.3/scala-2.12/

alfonsorr commented 1 year ago

Oh, ive just see that you are right, with the new format some final ones doesn't work https://www.habla.dev/doric/docs/api/spark-3.3/scala-2.12/doric/BinaryOperationsSyntax.html

Amalicia commented 1 year ago

Ah awesome! Thanks for confirming! At least thats one mystery solved - I'll have a look and see if I can dig any deeper into why things like BinaryOperationsSyntax are broken. I seem to have managed to make ArrayColumnsSyntax work locally so this might be a good starting point to try and work from 😄

eruizalo commented 1 year ago

Hi @Amalicia,

@alfonsorr has just found out what is happening here, or at least we think so. It seems it is related to access modifiers which we usually set as private[]

If we take a look at ArrayColumns traits we can see there are different access modifiers, so private traits are the ones not working while public traits work perfectly... image

Changing this to protected should do the trick.

Amalicia commented 1 year ago

Yep! Thats exactly the same thing I noticed 🎉 I mentioned it above as well but it wasn't worded in a wonderful way so I apologise for that 😅

I've moved all of the private[packages] to protected but am having a few compiler errors to work through, but hopefully I'll resolve them soon!

Amalicia commented 1 year ago

Hey again folks, I'm really sorry for the radio silence on this. Life has been a little crazy and hasn't left me with much time to follow up on this.

As an update, things are looking really close to being done. I am however having an issue with one of the links - slice to be exact. It's saying it can't find the member to link. I've been playing around with this and one thing I did notice was that it's trying to link to code available in Spark 3.1 however this slice function lives in the base package (i.e. Spark 2.4 if my understanding is correct). Therefore I've tried to update the link to what I assume if the correct one: def slice(x: Column, start: Int, length: Int) but this still doesn't seem to be working 😞

Is it possible either of you have some suggestions on what to look into to resolve this?

eruizalo commented 1 year ago

Hello again @Amalicia! Quite the opposite, we are really glad for your help whenever you have time and you feel like it.

As you pointed, we linked this function to its equivalent in Spark 2.4 because it was easier as the same link would work for every spark version, but the most accurate function to link would have been Spark 3.1. In the end the link is just an "extra" to help people about the relation doric-spark and I didn't see the point of duplicating the code just because of the documentation (this is the way I see this single point, but if you duplicated the code so the links would be more accurate I would not say a word about it)

I faced this very same error myself with some linksT there is an open issue related to this (#135) and some doric functions have a "TODO" task linked to this issue (@todo scaladoc link (issue #135)). I was not able to know what is happening there. I tried out many scenarios and I think those links worked in other projects, but, somehow, they are getting us some trouble in doric...

I think I saw some "working" links which stopped working after certain changes when I looked into this issue. You can push your branch and create a draft pull request if you want us to take a look about your changes and try to figure out what's happening.

If we could not solve this, I think we could go out without this link the same way we did before: add a TODO task linked to the issue.

Amalicia commented 1 year ago

Hello again! Sorry for the silence once again, life got in the way. I've continued working on and off on this but sadly haven't managed to get the links to work. I have raised a draft PR here so if there are any suggestions please let me know!

alfonsorr commented 1 year ago

Thanks, @Amalicia ! Your work is always welcome 😄