Initial implementation of Data.read_many
#11490
Conversation
radeusgd
requested review from
jdunkerley,
GregoryTravis,
AdRiley and
marthasharkey
as code owners
distribution/lib/Standard/Base/0.0.0-dev/src/System/File_Format.enso
Outdated
Show resolved
Hide resolved
# Conflicts: # distribution/lib/Standard/Table/0.0.0-dev/src/Table.enso
distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Read_Many_Helpers.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Read/Return_As.enso
Outdated
Show resolved
Hide resolved
| If `Auto_Detect` is specified; each file determines the specific | ||
| type and configures it appropriately. If there is no matching type then | ||
| a `File_Error.Unsupported_Type` error is returned. | ||
| - return: Specifies the shape of the data to return. |
There was a problem hiding this comment.
I'm not sure that I understand what a user would do with this option. Should it be on the graphical interface API?
There was a problem hiding this comment.
OK. I get what this is now. Guess we will improve the comment as we add more options.
There was a problem hiding this comment.
Happy to accept suggestions on what the doc could be. Perhaps I should add examples?
I hope in GUI it will be easy because we'll have a dropdown
radeusgd
added
CI: Ready to merge
distribution/lib/Standard/Base/0.0.0-dev/src/Data/Read/Many_Files_List.enso
Show resolved
Hide resolved
| to_display_text self -> Text = self.underlying.to_display_text | ||
|
|
||
| ## PRIVATE | ||
| make_return self (input : Many_Files_List) (objects : Vector Any) = |
There was a problem hiding this comment.
Why isn't this method private?
There was a problem hiding this comment.
It is used from within Standard.Table.
| Single_Choice display=Display.Always values=options | ||
|
|
||
| ## PRIVATE | ||
| type Return_As_Base |
There was a problem hiding this comment.
Why isn't this type in its own private module?
There was a problem hiding this comment.
Because its constructors are actually supposed to be visible, only the type is hidden. Perhaps it can be moved to a separate module.
distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Read_Many_Helpers.enso
Show resolved
Hide resolved
| package org.enso.base.read; | ||
|
|
||
| @org.openide.util.lookup.ServiceProvider(service = ReadManyReturnSPI.class) | ||
| public class BaseReadManyReturnSPI extends ReadManyReturnSPI { |
There was a problem hiding this comment.
Repeating the terminology misuse - the BaeReadManyReturn class isn't interface, this class is implementation. I suggest to rename to BaseReadManyReturnImpl.
Make final.
There was a problem hiding this comment.
All of our SPI implementations are named according to this convention.
If the convention is wrong, we should rename them all at once, which should be done separately from this PR.
| import org.enso.base.polyglot.EnsoMeta; | ||
| import org.graalvm.polyglot.Value; | ||
|
|
||
| public abstract class ReadManyReturnSPI { |
Pull Request Description
Data.read_many#11311Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.