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

Feature array separation #894

Closed

Conversation

AyrtonB
Copy link

@AyrtonB AyrtonB commented Aug 12, 2021


Overview

This PR aims to solve this issue around enabling delimiter-separated lists for array types

To Do:

  • Use delimiter specified in the field format
  • Apply the array_item types
  • Modify the general.json schema to allow any delimiter
  • Add in tests for the new array type functionality

Please preserve this line to notify @roll (lead of this repository)

@AyrtonB
Copy link
Author

AyrtonB commented Aug 12, 2021

I think this is the section which needs modifying in general.json to allow any delimiter.

"format": {
    "description": "There are no format keyword options for `array`: only `default` is allowed.",
    "enum": [
        "default"
    ],
    "default": "default"
},

By adding ', ' as an option to enum I can successfully load my array fields when using:

from frictionless import Package

package = Package('../data/dictionary/datapackage.json')
resource = package.get_resource('ids')

df = resource.to_pandas()

I'm confused though as to how I can generalise this to allow unknown separator types. I tried looking at the datetime type formats for inspiration and can see that they have a context entry, however, I'm unsure if that's what needs adding and if so what would need to be changed for the array type.

@roll
Copy link
Member

roll commented Aug 12, 2021

We can just drop enum there to allow any format value

@roll
Copy link
Member

roll commented Oct 12, 2021

Hi @AyrtonB,

do you need help with finishing this one?

@AyrtonB
Copy link
Author

AyrtonB commented Oct 13, 2021

Hi @roll. The issue I'm currently having is that I'm on windows which makes it a massive pain to run the test suite (have now tried several methods to get Make working including through WSL).

Is there an alternative command I can use to run the test-suite?

@roll
Copy link
Member

roll commented Oct 13, 2021

Hi @AyrtonB,

Sure, you can just use pytest. You can see all the commands it uses in Makefile

@thbar
Copy link

thbar commented Oct 28, 2021

@AyrtonB thanks for your work on this! We'll be happy to beta test this.

@AyrtonB
Copy link
Author

AyrtonB commented Oct 28, 2021

Thanks, I've been struggling to get the test suite working.

@AyrtonB
Copy link
Author

AyrtonB commented Oct 28, 2021

@thbar @roll FYI one of the commits isn't really related to the feature array problem, instead it's handling this use-case which currently leads to a package being failed to be read

@roll roll added the feature New functionality label Jan 10, 2022
@roll roll self-assigned this Apr 19, 2022
@roll
Copy link
Member

roll commented Nov 24, 2022

CLOSING this one as a stale one. We will work on a new PR for v5 when we're on #736

@roll roll closed this Nov 24, 2022
@johanricher
Copy link
Member

johanricher commented Nov 24, 2022

Hey roll, agreed to consider this PR as stale. This is still needed, the relevant issues are frictionlessdata/specs#736 and frictionlessdata/specs#381.

I'm available to help specify the implementation, or rethink it, if useful. :) We could also propose a new PR if you think this is not a priority for the next months or so.

I also take this opportunity to thank you again for your work, upcoming v5 looks like a great release! 🎉

@roll
Copy link
Member

roll commented Nov 28, 2022

Thanks @johanricher !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Improve describe/extract guides
4 participants