search

vegas-viz / Vegas

The missing MatPlotLib for Scala + Spark
MIT License
730 stars 98 forks source link

[proposal] Remove dependency on Argus #158

Closed oshikiri closed 5 years ago

oshikiri commented 6 years ago

vega-lite recently released v2.6.0 and v3.0.0-rc6. However, Vegas currently depends on vega-lite v1.2.0 because the Argus's anyOf issue (https://github.com/vegas-viz/Vegas/issues/83) blocks upgrading vega-lite to v1.2.1 (and the issue seems to be hard to resolve: https://github.com/aishfenton/Argus/issues/13).

If we remove dependency on Argus and then maintain Spec.scala manually, can we avoid the above issue and upgrade vega-lite to newer than v1.2.1?

I will try this if it is ok.

dankolesnikov commented 6 years ago

@oshikiri This will potentially help in #156 Is Spec.scala the only file that depends on Argus package? Are there other classes that use argus?

oshikiri commented 6 years ago

@dankolesnikov Thanks 😄

Is Spec.scala the only file that depends on Argus package?

core/src/test/scala/vegas/JsonMatchers.scala also uses Argus but it seems not to be critical. https://github.com/vegas-viz/Vegas/search?q=argus&unscoped_q=argus

dankolesnikov commented 6 years ago

@oshikiri Spec.scala seems to be a very large file to maintain. It seems logical to break it down into multiple classes for easier maintenance.

Is argus library semi-abandoned?

oshikiri commented 5 years ago

@dankolesnikov I'm not sure if I can (and should) remove Argus. (generating Spec.scala by Argus is better if we can resolve the anyOf issue)

I am trying to remove Argus and upgrade vega-lite at a sandbox branch, but now some tests are failing in my local environment. https://github.com/oshikiri/Vegas/compare/workspace-mkvegamodel...oshikiri:remove-argus

And, in the sandbox branch, there is still a dependency to argus at JsonMatchers.scala. Do you have any ideas for this? (ex. replacing argus.json.JsonDiff with something else, like for example scalatest-json)

aishfenton commented 5 years ago

I'm going to have a go at resolving the issue in Argus. Plan is to move Argus to Scalameta, and generally just make it more complete.

oshikiri commented 5 years ago

I came to think that automatically generation of Spec.scala is required because vega-lite-schema.json grows very fast; 2260 lines (v1.2.0) → 8932 lines (v2.6.0) → 10123 lines (v3.0.0-rc6).

@aishfenton Could you please provide more details of the plan? (Will it resolve https://github.com/aishfenton/Argus/issues/13?)

aishfenton commented 5 years ago

Yes, Basically working now on updating Argus to support newer versions of JSON Schema, and in general be more maintainable (move from Macros to Scalameta mostly). Then we should be able to just generate a vega-lite schema for v3 again, and hopefully update it more frequently from then on.

On Sat, Oct 20, 2018 at 1:17 AM Takamasa Oshikiri notifications@github.com wrote:

I came to think that automatically generation of Spec.scala is required because vega-lite-schema.json grows very fast; 2260 lines (v1.2.0 https://github.com/vega/vega-lite/blob/v1.2.0/vega-lite-schema.json) → 8932 lines (v2.6.0 https://github.com/vega/vega-lite/blob/v2.6.0/build/vega-lite-schema.json) → 10123 lines (v3.0.0-rc6 https://github.com/vega/vega-lite/blob/v3.0.0-rc6/build/vega-lite-schema.json ).

@aishfenton https://github.com/aishfenton Could you please provide more details of the plan? (Is it related to aishfenton/Argus#13 https://github.com/aishfenton/Argus/issues/13?)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vegas-viz/Vegas/issues/158#issuecomment-431559974, or mute the thread https://github.com/notifications/unsubscribe-auth/AAM_LB9ZRuW8tKWbmxBrPBHgICvhKfIMks5umtwugaJpZM4XCPeW .

oshikiri commented 5 years ago

@aishfenton Thanks. I think your plan is better.

P.S. Could you please check https://github.com/vegas-viz/Vegas/issues/138#issuecomment-429370077?