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

#159, #72 - Restructuring of docs #436

Merged
merged 12 commits into from
Aug 2, 2018
Merged

Conversation

schillic
Copy link
Member

@schillic schillic commented Jul 30, 2018

Closes #159. Workaround for #72.

  • Add missing documentation.
  • Add workaround for Fix 'docstrings potentially missing' warnings #72. The only warnings that are still printed are the ones for functions only defined in presence of Polyhedra.jl. Should we remove them as well?
  • Have interface documentation at one place and link to it (Remove duplicate inherited functions from docs #159).
    • There are still some problems with Documenter in case of multiple methods. As a workaround, I omit the hyperlinks when there are conflicts.
    • To elaborate:
      • default values (e.g., p::Real=Inf): This simply does not work.
      • type parameters: On the lowest interface level we can use an instantiation of the type parameters, like an_element(::LazySet{Real}); for this case one can also create a hyperlink reference. On higher (interface/type) levels, one has to use the where syntax in order to use the correct method; for this case there are no hyperlinks.
  • Remove duplicate documentation for concrete types.
    • See above (where syntax).
    • I did not find a way to make the array function for array types (probably due to nested type parameters) and the functions with default values (norm/radius/diameter) work.
  • Check that each implementation occurs in the docs.
  • Check that workaround for Fix 'docstrings potentially missing' warnings #72 is not created by default.
    • The page is not created, but the warnings are absorbed.
  • Fix broken doctests.
    • I have never seen those broken ones so far. Apparently up to now the doctests never executed for VPolytope.jl and plot_recipes.jl, probably because the documentation was not used before.

@schillic schillic force-pushed the schillic/docs_restructure branch 2 times, most recently from 8deca4d to 77e9eec Compare July 30, 2018 21:55
@@ -189,6 +189,10 @@ Depth = 2

## Library Outline

```@docs
LazySets
Copy link
Member

Choose a reason for hiding this comment

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

just a comment: many people do not add a docstring to module LazySets, in which case this @docs is not needed -- by default julia will print the README.md of the package. i like this idea because it has some more info such as links to online documentation that our current Main module for LazySets.jl -- a Julia package for calculus with convex sets.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, then I vote for removing this @docs. Otherwise we get a warning.

Copy link
Member

Choose a reason for hiding this comment

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

okay. can you do that?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,41 @@
# Functions with several methods
Copy link
Member

Choose a reason for hiding this comment

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

what is the purpose of the file methods_fix.md?

Copy link
Member Author

@schillic schillic Jul 31, 2018

Choose a reason for hiding this comment

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

If you add it, all the Documenter warnings about missing documentation are gone. Of course this is dangerous, so I commented this line. But it can help find missing docs at least for new function/type names quickly. In particular, I finally managed to exclude the warnings about apply_recipe!

Copy link
Member

Choose a reason for hiding this comment

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

hooray 🙌

most if not all of these functions with several methods should be documented, such as constraints_list. why does Documenter complain? since we include some (maybe not all) the variations of constraints_list in the docs.

Copy link
Member Author

@schillic schillic Jul 31, 2018

Choose a reason for hiding this comment

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

No, the reason is described in #72. Documenter just does not recognize methods with default values or where syntax. Currently there is just no other way than manually searching for those docs. That is why I said that it is dangerous 😟

@schillic schillic force-pushed the schillic/docs_restructure branch 14 times, most recently from 1753060 to 1d9b491 Compare August 1, 2018 21:18
@schillic schillic changed the title [WIP #159, #72] Restructuring of docs #159, #72 - Restructuring of docs Aug 1, 2018
@schillic schillic requested a review from mforets August 1, 2018 21:47
Copy link
Member

@mforets mforets left a comment

Choose a reason for hiding this comment

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

Titanic changeset, thanks for fixing!! 😄

here i only get:

 > checking for missing docstrings.
 !! 6 docstrings potentially missing:

    LazySets.cartesian_product :: Tuple{LazySets.VPolytope,LazySets.VPolytope}
    LazySets.cartesian_product :: Tuple{LazySets.HPolytope,LazySets.HPolytope}
    Polyhedra.polyhedron :: Union{Tuple{LazySets.VPolytope{N},Any}, Tuple{LazySets.VPolytope{N}}, Tuple{N}} where N
    Polyhedra.polyhedron :: Union{Tuple{LazySets.HPolytope{N},Any}, Tuple{LazySets.HPolytope{N}}, Tuple{N}} where N
    Base.intersect :: Union{Tuple{LazySets.VPolytope{N},LazySets.VPolytope{N}}, Tuple{N}} where N<:Real
    Base.intersect :: Union{Tuple{LazySets.HPolytope{N},LazySets.HPolytope{N}}, Tuple{N}} where N<:Real

@schillic
Copy link
Member Author

schillic commented Aug 2, 2018

Yes, these are only there with Polyhedra.
It is really cheating here. I just define a file that absorbs all other warnings. I am really not sure if this is a good idea or not. At least we can spot problems with new functions again. To me the previous state was at least as useless...

@schillic schillic merged commit aea6990 into master Aug 2, 2018
@schillic schillic deleted the schillic/docs_restructure branch August 2, 2018 19:13
@schillic schillic mentioned this pull request Aug 3, 2018
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.

None yet

2 participants