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

Suggest to open Bigarray when using its indexing operators. #299

Closed
wants to merge 1 commit into from

Conversation

alainfrisch
Copy link
Contributor

This PR implements the proposal from http://caml.inria.fr/mantis/view.php?id=6765 . Now that .{}-like operators are user-definable, one needs to explicitly open Bigarray. To help with the transition, this PR adapts the "Unbound value" error printer to suggest opening Bigarray.

If #248 is accepted, one should change the text of the suggestion.

Note that on Mantis, it was also discussed to add a hack to automatically lookup those operators in Bigarray and issue a warning. Perhaps we should accept the patch there and keep this PR for later (when the hack is removed).

@Octachron
Copy link
Member

Personnally, I am rather in favor of the temporary lookup hack for 4.03 and then a transition to an error and a custom warning next version. It could make the transition smoother than going directly to an error.
I would try to submit on github my updated patch for the temporary hack tonight.

However, a problem with both patchs is that the error message is not entirely convincing: should you recommend to open the bigarray module directly? There was sensible complaints in mantis:6765 and #248 that opening the Bigarray module is not good practice and should not be encouraged.

Maybe it would make more sense to also link the relevant section of the reference manual for a more detailled explanation of what is going on?

@gasche
Copy link
Member

gasche commented Nov 20, 2015

The current plan is to not include #69 in the next release, making this one unnecessary, so I will close it to facilitate triaging. I do plan to keep #69 available in a branch, and in this branch we should also apply the "temporary lookup hack" that we decided to have for backward-compatibility (before making the revert decision). So Pull Requests to implement this are warmly welcome. (I think I'll also include the module Operators = ... PR in this branch.

@gasche gasche closed this Nov 20, 2015
@Octachron
Copy link
Member

I like the idea of integrating the operators submodule patch in the branch: it makes both the documentation and the deprecation warning cleaner. I will submit a pull request to implement the temporary deprecation warning once there is a clear place to do so. As an alternative possibility, I could also merge both commits in #69 after the reversion.

@gasche
Copy link
Member

gasche commented Nov 20, 2015

While we're at it, another thing not to forget is to revert your (excellent) documentation patch for indexing operators. I looks like #302 is progressing faster than I expected, so I'll wait for it to be merged and put #69 and ocaml/ocaml-manual#8 in the same branch (easier than having two related branches in two distinct repositories).

mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Feb 24, 2021
* On export, mark all variables as having mode In_types in Aliases

* Add fast path for simplification of Project_var

* Fix use of closure vars

* Row_like: add exception handler
poechsel pushed a commit to poechsel/ocaml that referenced this pull request Mar 1, 2021
* On export, mark all variables as having mode In_types in Aliases

* Add fast path for simplification of Project_var

* Fix use of closure vars

* Row_like: add exception handler
poechsel pushed a commit to poechsel/ocaml that referenced this pull request Mar 1, 2021
* On export, mark all variables as having mode In_types in Aliases

* Add fast path for simplification of Project_var

* Fix use of closure vars

* Row_like: add exception handler
chambart pushed a commit to chambart/ocaml-1 that referenced this pull request Jan 4, 2022
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
* Add hello_world script

* Delete description in esy.json
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Co-authored-by: Mirza Babar <mirza.babar35@gmail.com>
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

3 participants