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
Moving away from Integer | Nothing
to Rows_To_Read
for limiting number of rows.
#9925
Conversation
Done `Table`, `Column`, `DB_Table` and `DB_Column`.
Move some of the functionality into the `Rows_To_Read` type.
6a6e981
to
2521426
Compare
_ -> input | ||
|
||
## PRIVATE | ||
Rows_To_Read.from (that:Nothing) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these here only to handle existing code that uses the old interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep exactly. Any old limits will then be ok. Though not if the warning flag was set.
distribution/lib/Standard/Table/0.0.0-dev/src/Rows_To_Read.enso
Outdated
Show resolved
Hide resolved
Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>
read self (max_rows : Nothing | Integer = Nothing) (warn_if_more_rows:Boolean = True) = | ||
@max_rows Rows_To_Read.default_widget | ||
read : Rows_To_Read -> Column | ||
read self (max_rows : Rows_To_Read = ..All_Rows) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ..All_Rows
looks so much clearer than Nothing
. I'm really happy with our switching towards more meaningful arguments. And with autoscoping it is still quite terse :)
## PRIVATE | ||
attach_warning self input:Table -> Table = case self of | ||
Rows_To_Read.First_With_Warning rows -> if input.row_count <= rows then input else | ||
Problem_Behavior.Report_Warning.attach_problem_after (input.take (First rows)) <| | ||
Not_All_Rows_Downloaded.Warning rows | ||
_ -> input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice that this is now in a single place, makes the usages much clearer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome
Pull Request Description
Rows_To_Read
type with conversions fromNothing
and integers.read
onTable
,Column
,DB_Table
andDB_Column
.Delimited_Format.Delimited
to useRows_To_Read
forrow_limit
.Excel_Format.Sheet
andExcel_Format.Range
to useRows_To_Read
forrow_limit
.Excel_Workbook.read
to useRows_To_Read
.Connection.read
(in all connection types) to useRows_To_Read
.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.