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.

Sign up for GitHub

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

Jump to bottom

IO Package refactor #172

Open
wants to merge 63 commits into
base: develop
Choose a base branch
from
Open

IO Package refactor #172

wants to merge 63 commits into from

Conversation

javihern98
Copy link
Contributor

@javihern98 javihern98 commented Jan 13, 2025 •
edited
Loading

This PR is a combination of #164 and #156 with many consistency checks. It covers the whole IO package for better consistency

Closes #164
Closes #156

Summary of changes

Dependency management

  • Removed extra FMR as we make httpx a mandatory dependency (we need it as well in IO package to download files from URL in read_sdmx)
  • Updated Poetry.lock

IO package

Consistency checks (#164)

  • Changed signature from readers and writers as specified in Checking for inconsistencies in parsers and writers methods #164. Normalization across the whole IO package to use Sequence
  • Changed short_urn to be a property on Maintainable Artefact
  • Ensure we use Dataflow=... and DataStructure=... across the IO package as short_urn for Schemas
  • Removed MessageType and used SDMXFormat on writers
  • Updated signature of input processor to use same terminology all across the functions

Convenience methods (#156)

  • Changed input processor to infer SDMX format and flavor (subformat, like SDMX-ML 2.1 Generic)
  • Added SDMXFormat enumeration with supported flavors
  • Added read_sdmx as a general reader for Path, URL or Strings
  • Added get_datasets to extract the Sequence of Datasets with the possibility to use a structures file to extract the Schema

Model package

  • Moved ActionType to Dataset (makes sense only here)
  • Cleaned up Message module and used same typing as readers and writers
  • Moved SubmissionResult (from RegistryInterface) to submission.py
  • Tag on Concept and Item Reference (Util package) due to the use of Concept Identity of both of them in the Component class

Tests changes

  • Adapted tests to the changes above and removed extra tests. Code Coverage reached at 100%

javihern98 and others added 30 commits December 18, 2024 13:23
@javihern98
Draft code for read_sdmx.
3664415 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Merge branch 'develop' into 156-add-read_sdmx-convenience-method
bdf8145
@javihern98
Refactored code on Message class to move submission and ActionType to…
2d87af7 
… their classes. Added read_sdmx method to reader

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Linting and mypy changes
617aa4c 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Refactored code to ensure we use the Short URN as keys of the message…
2c2cab3 
… content. Use ItemReference as ConceptIdentity if necessary. Fixed tests accordingly.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Added tests for read_sdmx with csv files.
ffb2a8e 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Linting and mypy changes.
01f5489 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Adapted structures tests to reach max code coverage
ab63f01 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Removed unique_id method and adapted code to use Reference.
8b960d2 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Added utils methods and classes to all.
41e7a8d 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Adapted tests for input processor to max code coverage.
b140de0 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Adapted tests for read_sdmx to max code coverage.
0c03933 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Added tag to concept and itemReference to avoid serialization issues.
f567c50 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Merge remote-tracking branch 'refs/remotes/origin/develop' into 156-a…
85076af 
…dd-read_sdmx-convenience-method

# Conflicts:
#	src/pysdmx/model/message.py
@javihern98
Fixed message imports to prevent circular imports. Ruff automatic fix…
fe1fbb4 
…es. Added get_datasets methods. Maxed code coverage in all code.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@mla2001
Read and write XML methods added (as requested, the write method has …
8afd68b 
…been split into write data and metadata). Read and write XML tests updated to work with the new methods.
@mla2001
Fixed ruff errors.
5417e73
@mla2001
Fixed mypy errors.
6e9ec54
@mla2001
Fixed read Path input error.
386af2f
@javihern98
Adapted ReadSDMX to infer the format automatically.
6b6886f 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Linting changes.
b6e6200 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Renamed ReadFormat to SDMXFormat. Added Submission and Error formats …
03dd8ad 
…detection on SDMX-ML. Fixed tests.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Ignored mypy error.
bd27711 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Added to_schema method to DataStructureDefinition. Draft code for get…
5818c40 
…_datasets method.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Replaced whole URN to Short URN in Dataset. Added tests to match cove…
6a4bac7 
…rage.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Added get_datasets to io
d38ed7f 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Added URL parsing support on read_sdmx. Made httpx a mandatory depend…
e2b2946 
…ency.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Merge branch 'develop' into 156-add-read_sdmx-convenience-method
4eca815
@javihern98
Updated poetry lock.
183fe4d 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Merge remote-tracking branch 'origin/156-add-read_sdmx-convenience-me…
d8fa929 
…thod' into 156-add-read_sdmx-convenience-method
@javihern98
Refactor on method names to make it private, signature of structures …
85bc5ea 
…writing and removing redundancies

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Merge remote-tracking branch 'refs/remotes/origin/164-checking-for-in…
f6df667 
…consistencies-in-parsers-and-writers-methods' into io_refactor

# Conflicts:
#	src/pysdmx/io/xml/sdmx21/writer/structure.py
#	tests/io/xml/sdmx21/reader/test_reader.py
#	tests/io/xml/sdmx21/writer/test_structures_writing.py
@javihern98
Refactor on message to use Sequence[...] objects.
53ac3e6 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Draft code on SDMX-ML 2.1 readers refactor to be consistent with writ…
f78463c 
…ers file structure.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Draft code on input processing. Added dataflow support to get_datasets.
a527de0 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Removed inconsistencies on read signature. Fixed input data on each X…
09fbb8c 
…ML reader. Added some error handling if a wrong SDMX message is used as input.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Fixed mypy errors before testing.
af4ecc7 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Changed short_urn to property in MaintainableArtefact.
f84d234 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Added property short_urn to DataStructureDefinition (sdmxtype must be…
a9af618 
… DataStructure). Changed Submission to RegistryInterface. Added better documentation for structures formats. Fixed tests for io and csv.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Fixed tests on xml.
4cfcab7 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Fixed message tests.
2363a61 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Removed inconsistencies on SDMX-CSV writers by adding the datasets as…
885f1af 
… a list.

Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Cleaned up code. Added tests for Coverage.
3a18798 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98 javihern98 self-assigned this Jan 13, 2025
This was referenced Jan 13, 2025
Closed
Closed
@javihern98
Replaced MessageType to SDMXFormat. Fixed tests.
3cbde9f 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Mypy fix.
8105528 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 13, 2025
View reviewed changes
src/pysdmx/io/xml/sdmx21/writer/structure.py Dismissed Show dismissed Hide dismissed
@javihern98
Docs updated
b16b2c6 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Updated signature on input processor
fd1c3f3 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98 javihern98 marked this pull request as ready for review January 13, 2025 16:52
@javihern98 javihern98 mentioned this pull request Jan 14, 2025
Open
sosna
sosna reviewed Jan 16, 2025
View reviewed changes
src/pysdmx/io/enums.py Outdated
SDMX_ML_2_1_DATA_GENERIC = "SDMX-ML 2.1 Generic"
SDMX_ML_2_1_REGISTRY_INTERFACE = "SDMX-ML 2.1 Registry Interface"
SDMX_ML_2_1_ERROR = "SDMX-ML 2.1 Error"
SDMX_JSON_2 = "SDMX-JSON 2.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should also distinguish between Structure and Data, as we do with XML?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, let me check the Standard, in any case this has no impact in the rest of the code as we have a NotImplemented

Copy link
Contributor Author

@javihern98 javihern98 Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Agreed to differentiate between SDMX_JSON_2_DATA and SDMX_JSON_2_STRUCTURE

src/pysdmx/io/enums.py Show resolved Hide resolved
src/pysdmx/model/dataflow.py
@@ -439,6 +440,23 @@ class DataStructureDefinition(MaintainableArtefact, frozen=True, kw_only=True):

components: Components

def to_schema(self) -> Schema:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this necessary? The reason Schema was introduced was to represent the response of an SDMX-REST Schema query. In this context, it contains very useful pieces of information like the artefacts property, which should list the URNs of all the artefacts that have been considered to generate the schema. If you don't provide this piece of information, then I don't see what converting to a Schema brings as benefits... Just a question :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is necessary to have a simple interface between the DataStructureDefinition and Schema. For sure then I will need to fill the artefacts as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we discuss this? Thanks :)

Copy link
Contributor Author

@javihern98 javihern98 Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

After discussion:

  • I will add the artefacts when generating the Schema. This includes the Concepts, Codelists and DataConstraints (not implemented as of today) of the DataStructureDefinition
  • We need to close the gap as much as we can between the Schema and DataStructureDefinition, Dataflow and ProvisionAgreement to make these classes interchangeable. This will be addressed in future releases

src/pysdmx/model/submission.py Show resolved Hide resolved
src/pysdmx/io/csv/sdmx10/writer/__init__.py Outdated Show resolved Hide resolved
@javihern98
Fixed typo on write function in csv.
d14d5db 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
@javihern98
Separated enumeration of SDMX JSON into Data and Structures
f8e2815 
Signed-off-by: javier.hernandez <javier.hernandez@meaningfuldata.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sosna sosna sosna left review comments

@stratosn stratosn Awaiting requested review from stratosn

@antonio-olleros antonio-olleros Awaiting requested review from antonio-olleros

At least 1 approving review is required to merge this pull request.

Assignees

@javihern98 javihern98

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Checking for inconsistencies in parsers and writers methods Add read_sdmx convenience method
3 participants
@javihern98 @sosna @mla2001