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

Add list of breaking changes since sssom release 0.3.31 #461

Open
matentzn opened this issue Nov 13, 2023 · 10 comments
Open

Add list of breaking changes since sssom release 0.3.31 #461

matentzn opened this issue Nov 13, 2023 · 10 comments
Assignees

Comments

@matentzn
Copy link
Collaborator

We need a list of breaking changes since sssom release 0.3.31, to communicate this with stakeholders before the 0.4 release.

@cthoyt my assumption is you dont want to do this, right?

@hrshdhgd
Copy link
Contributor

hrshdhgd commented Nov 20, 2023

Active list being updated:

  • from sssom.context import get_default_metadata => from sssom.constants import get_default_metadata
  • from sssom.typehints import Metadata => from sssom.constants import MetadataType
    • Older Metadata had the metadata attribute which is now collapsed
      • e.g. meta: Metadata
                mapping_set_id = meta.metadata[MAPPING_SET_ID]
                license = meta.metadata[LICENSE]
        
        is now meta: MetadataType
                mapping_set_id = meta[MAPPING_SET_ID]
                license = meta[LICENSE]
        
  • doc = MappingSetDocument(prefix_map=metadata[PREFIX_MAP_KEY], mapping_set=mset)
    •   converter = curies.chain([
           Converter.from_prefix_map(metadata.pop(CURIE_MAP, {})),
           ensure_converter(prefix_map, use_defaults=False)
       ])
        doc = MappingSetDocument(converter=converter, mapping_set=mset)
      
  • metadata = get_metadata_and_prefix_map(MATCHER_META_YML) => converter, metadata = get_metadata_and_prefix_map(MATCHER_META_YML)

@hrshdhgd
Copy link
Contributor

The above were the refactors for oaklib. For the rest, Charlie knows best...

@matentzn
Copy link
Collaborator Author

Fantastic! Is there a way to get a list of all functions that

  1. changed name
  2. changed package
  3. changed parameter list

using some tool?

@cthoyt
Copy link
Member

cthoyt commented Nov 21, 2023

Reading through v0.3.41...v0.4.0-rc1 is not fun but is one way to do it. Maybe also contextualizing with the various PR descriptions

@matentzn
Copy link
Collaborator Author

@hrshdhgd I made a quick diff to get a sense of some of the more important changes:

-def validate(input: str, validation_types: tuple):
+def validate(input: str, validation_types: List[SchemaValidationType]):
-def crosstab(input: str, output: TextIO, transpose: bool, fields: Tuple):
+def crosstab(input: str, output: TextIO, transpose: bool, fields: Tuple[str, str]):
-def correlations(input: str, output: TextIO, transpose: bool, fields: Tuple):
+def correlations(input: str, output: TextIO, transpose: bool, fields: Tuple[str, str]):
-def get_jsonld_context():
-def get_extended_prefix_map():
-def get_built_in_prefix_map() -> PrefixMap:
+def _get_built_in_prefix_map() -> Converter:
-def add_built_in_prefixes_to_prefix_map(
+def ensure_converter(prefix_map: ConverterHint = None, *, use_defaults: bool = True) -> Converter:
-def get_default_metadata() -> Metadata:
-def _raise_on_invalid_prefix_map(prefix_map):
-def set_default_mapping_set_id(meta: Metadata) -> Metadata:
-def set_default_license(meta: Metadata) -> Metadata:
-def _get_prefix_map(metadata: Metadata, prefix_map_mode: str = None):
-def get_list_of_predicate_iri(predicate_filter: tuple, prefix_map: dict) -> list:
+def get_list_of_predicate_iri(predicate_filter: Iterable[str], converter: Converter) -> list:
-def extract_iri(input: str, prefix_map: Dict[str, str]) -> List[str]:
+def extract_iris(
-def read_sssom_table(
-def read_sssom_rdf(
-def read_sssom_json(
-def _cell_element_values(cell_node, converter: Converter, mapping_predicates) -> Optional[Mapping]:
+def _cell_element_values(cell_node, converter: Converter, mapping_predicates) -> Dict[str, Any]:
-def sha256sum(path: str) -> str:
-def read_csv(
-def read_metadata(filename: str) -> Metadata:
-def read_pandas(file: Union[str, Path, TextIO], sep: Optional[str] = None) -> pd.DataFrame:
-def extract_global_metadata(msdoc: MappingSetDocument) -> Dict[str, PrefixMap]:
+def _extract_global_metadata(msdoc: MappingSetDocument) -> MetadataType:
-def get_prefixes_used_in_table(df: pd.DataFrame) -> List[str]:
+def get_prefixes_used_in_table(df: pd.DataFrame, converter: Converter) -> Set[str]:
-def guess_file_format(filename: Union[str, TextIO]) -> str:
-def get_all_prefixes(msdf: MappingSetDataFrame) -> list:
+def get_all_prefixes(msdf: MappingSetDataFrame) -> Set[str]:
-def to_dataframe(msdf: MappingSetDataFrame) -> pd.DataFrame:

A big red flag is:

-def read_sssom_table(
-def read_sssom_rdf(
-def read_sssom_json(
-def read_csv(
-def read_metadata(filename: str) -> Metadata:
-def read_pandas(file: Union[str, Path, TextIO], sep: Optional[str] = None) -> pd.DataFrame:

-> we should never delete public methods, always deprecate them. Can we chase down why this happened, and reinstate them (wrapping their replacements)?

@cthoyt
Copy link
Member

cthoyt commented Nov 21, 2023

I think all of the public methods deleted were marked as deprecated and the version where they were marked to be removed had already passed.

@cthoyt
Copy link
Member

cthoyt commented Nov 21, 2023

The minuses could have also happened if better type annotations got put and the defintion spilled over onto multiple lines. I don't think there are any places we removed non-expired code from the public interface

@hrshdhgd
Copy link
Contributor

hrshdhgd commented Nov 21, 2023

  • read_sssom_table => parse_sssom_table
  • read_sssom_rdf => parse_sssom_rdf
  • read_sssom_json => parse_sssom_json
  • read_csv => September cleaning 1 #413 (I think)
  • read_metadata => _read_pandas_and_metadata
  • read_pandas => _read_pandas_and_metadata

And Charlie is right, the deprecations were long overdue

@matentzn
Copy link
Collaborator Author

Ok thanks for checking!

@matentzn
Copy link
Collaborator Author

@hrshdhgd I will leave it to you to move this forward then!

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

No branches or pull requests

3 participants